i18n review

Hi all,

I almost have the i18n review also finish but I’ve found several issues that I’ll leave unfix because I think they need further changes in the code.

We should avoid, as much as possible, comparisions (or other logic) relaying directly in strings shown at user interface. For example:

wsList.setModel(new WsUrlTableModel(tdat));
wsList.getColumn(“Status”).setMinWidth(10);

[…]

public String getColumnName(int column)
{
if (column == 1)
{
return “Status”;
}
return “Service URL”;
}

If the string gets translated, the application will not work properly (or even won’t work at all).

I can fix these type of issues but, from my point of view, it’s better to them in the main branch instead of the i18n branch, because they will need not-i18n-changes before to fix i18n. What do you think about that?

Cheers,

David

The point of the i18n branch is to allow independent patching and merging. I suggest you add a TODO

// TODO: JAL-1354 do not index with i18n strings

… in fact – raise a bug and cite the bug in the TODO. One of us will look at it as we merge in the i18n changes.

I’m still looking at your last commits – unfortunately, I’m dealing with some stuff at the moment which means I won’t have finished reviewing them until Monday. Don’t let this stop you !

Jim

···

Hi all,

I almost have the i18n review also finish but I’ve found several issues that I’ll leave unfix because I think they need further changes in the code.

We should avoid, as much as possible, comparisions (or other logic) relaying directly in strings shown at user interface. For example:

wsList.setModel(new WsUrlTableModel(tdat));
wsList.getColumn(“Status”).setMinWidth(10);

[…]

public String getColumnName(int column)
{
if (column == 1)
{
return “Status”;
}
return “Service URL”;
}

If the string gets translated, the application will not work properly (or even won’t work at all).

I can fix these type of issues but, from my point of view, it’s better to them in the main branch instead of the i18n branch, because they will need not-i18n-changes before to fix i18n. What do you think about that?

Cheers,

David

ok then. I’ll make a complete review and fix.
which deadline are we working with? I mean, not release date but the date you would like to have the fixes ready to start merging process.

···

El 20/09/2014 19:23, “James Procter” <J.Procter@dundee.ac.uk> escribió:

The point of the i18n branch is to allow independent patching and merging. I suggest you add a TODO

// TODO: JAL-1354 do not index with i18n strings

… in fact – raise a bug and cite the bug in the TODO. One of us will look at it as we merge in the i18n changes.

I’m still looking at your last commits – unfortunately, I’m dealing with some stuff at the moment which means I won’t have finished reviewing them until Monday. Don’t let this stop you !

Jim

From: jalview-dev-bounces@jalview.org [mailto:jalview-dev-bounces@jalview.org] On Behalf Of David Roldán Martínez
Sent: 20 September 2014 17:50
To: Jalview Development List
Subject: [Jalview-dev] i18n review

Hi all,

I almost have the i18n review also finish but I’ve found several issues that I’ll leave unfix because I think they need further changes in the code.

We should avoid, as much as possible, comparisions (or other logic) relaying directly in strings shown at user interface. For example:

wsList.setModel(new WsUrlTableModel(tdat));
wsList.getColumn(“Status”).setMinWidth(10);

[…]

public String getColumnName(int column)
{
if (column == 1)
{
return “Status”;
}
return “Service URL”;
}

If the string gets translated, the application will not work properly (or even won’t work at all).

I can fix these type of issues but, from my point of view, it’s better to them in the main branch instead of the i18n branch, because they will need not-i18n-changes before to fix i18n. What do you think about that?

Cheers,

David

The University of Dundee is a registered Scottish Charity, No: SC015096


Jalview-dev mailing list
Jalview-dev@jalview.org
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev

Hello David,

My comments on JAL-1428 included

  1. things to change in / add to the Spanish message bundle
  2. new externalised strings that could be added in the code, English and Spanish bundles

Do you want me to address (1) and shall I look at (2)?

Thanks,

Mungo

···

On Sun, Sep 21, 2014 at 11:56 AM, David Roldán Martínez <darolmar@gmail.com> wrote:

ok then. I’ll make a complete review and fix.
which deadline are we working with? I mean, not release date but the date you would like to have the fixes ready to start merging process.

El 20/09/2014 19:23, “James Procter” <J.Procter@dundee.ac.uk> escribió:

The point of the i18n branch is to allow independent patching and merging. I suggest you add a TODO

// TODO: JAL-1354 do not index with i18n strings

… in fact – raise a bug and cite the bug in the TODO. One of us will look at it as we merge in the i18n changes.

I’m still looking at your last commits – unfortunately, I’m dealing with some stuff at the moment which means I won’t have finished reviewing them until Monday. Don’t let this stop you !

Jim

From: jalview-dev-bounces@jalview.org [mailto:jalview-dev-bounces@jalview.org] On Behalf Of David Roldán Martínez
Sent: 20 September 2014 17:50
To: Jalview Development List
Subject: [Jalview-dev] i18n review

Hi all,

I almost have the i18n review also finish but I’ve found several issues that I’ll leave unfix because I think they need further changes in the code.

We should avoid, as much as possible, comparisions (or other logic) relaying directly in strings shown at user interface. For example:

wsList.setModel(new WsUrlTableModel(tdat));
wsList.getColumn(“Status”).setMinWidth(10);

[…]

public String getColumnName(int column)
{
if (column == 1)
{
return “Status”;
}
return “Service URL”;
}

If the string gets translated, the application will not work properly (or even won’t work at all).

I can fix these type of issues but, from my point of view, it’s better to them in the main branch instead of the i18n branch, because they will need not-i18n-changes before to fix i18n. What do you think about that?

Cheers,

David

The University of Dundee is a registered Scottish Charity, No: SC015096


Jalview-dev mailing list
Jalview-dev@jalview.org
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev


Jalview-dev mailing list
Jalview-dev@jalview.org
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev

​David,

  • ignore the message below. I’ve just seen your comment added on Friday. I will check out the latest in the branch.

thanks

mungo

The University of Dundee is a registered Scottish Charity, No: SC015096

···

From: jalview-dev-bounces@jalview.org jalview-dev-bounces@jalview.org on behalf of Mungo Carstairs gmungoc@gmail.com
Sent: 29 September 2014 11:15
To: Jalview Development List
Subject: Re: [Jalview-dev] i18n review

Hello David,

My comments on JAL-1428 included

  1. things to change in / add to the Spanish message bundle
  2. new externalised strings that could be added in the code, English and Spanish bundles

Do you want me to address (1) and shall I look at (2)?

Thanks,

Mungo

On Sun, Sep 21, 2014 at 11:56 AM, David Roldán Martínez <darolmar@gmail.com> wrote:

ok then. I’ll make a complete review and fix.
which deadline are we working with? I mean, not release date but the date you would like to have the fixes ready to start merging process.

El 20/09/2014 19:23, “James Procter” <J.Procter@dundee.ac.uk> escribió:

The point of the i18n branch is to allow independent patching and merging. I suggest you add a TODO

// TODO: JAL-1354 do not index with i18n strings

… in fact – raise a bug and cite the bug in the TODO. One of us will look at it as we merge in the i18n changes.

I’m still looking at your last commits – unfortunately, I’m dealing with some stuff at the moment which means I won’t have finished reviewing them until Monday. Don’t let this stop you !

Jim

From: jalview-dev-bounces@jalview.org [mailto:jalview-dev-bounces@jalview.org] On Behalf Of David Roldán Martínez
Sent: 20 September 2014 17:50
To: Jalview Development List
Subject: [Jalview-dev] i18n review

Hi all,

I almost have the i18n review also finish but I’ve found several issues that I’ll leave unfix because I think they need further changes in the code.

We should avoid, as much as possible, comparisions (or other logic) relaying directly in strings shown at user interface. For example:

wsList.setModel(new WsUrlTableModel(tdat));
wsList.getColumn(“Status”).setMinWidth(10);

[…]

public String getColumnName(int column)
{
if (column == 1)
{
return “Status”;
}
return “Service URL”;
}

If the string gets translated, the application will not work properly (or even won’t work at all).

I can fix these type of issues but, from my point of view, it’s better to them in the main branch instead of the i18n branch, because they will need not-i18n-changes before to fix i18n. What do you think about that?

Cheers,

David

The University of Dundee is a registered Scottish Charity, No: SC015096


Jalview-dev mailing list
Jalview-dev@jalview.org
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev


Jalview-dev mailing list
Jalview-dev@jalview.org
http://www.compbio.dundee.ac.uk/mailman/listinfo/jalview-dev