jalview

Clone Tools

Stats

Commits this week: 17
Total Committers: 73
Last Commit:
Commits this week: 17

Lines of code count not available

Commit Activity

Commits By Day In Week
52 week commits volume
Commits By Day In Week
Commits by day
Commits By Day In Week
Commits by hour
Commits By Hour In Day
Commit calendar
 

Most active committers (90 days)

loading
loading
  • last updated a few seconds ago
All Commits Reviews
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Monday 08 Aug
6:31 pm

Jim Procter

Friday 22 Jul
4:40 pm

Ben Soares

I've now put in a warn with the e.getMessage(), but I've left in a debug with the full stacktrace. This catch block is a little odd in that it's used as programmatic flow for non 200 HTTP responses...

I've now put in a warn with the e.getMessage(), but I've left in a debug with the full stacktrace.
This catch block is a little odd in that it's used as programmatic flow for non 200 HTTP responses. I notice that the new API server is sending back a 400 for bad input (including for non-existent query fields and wrong types for non-string query fields), which is invoking the warning triangle which puts the HTTP error message into the tooltip so this block is now getting more use!

The reason for wanting a full stacktrace in debug mode is that both the API and the changed client are new, so any errors in the next few months will be more quickly diagnosed if we can allow the user to easily obtain a stacktrace of the exception.

12:34 pm

Jim Procter

after trying it out I think the old behaviour is better, unless 1) there's a really clear message in the UI explaining that now the field has changed, the search needs to be executed again 2) a con...

after trying it out I think the old behaviour is better, unless 1) there's a really clear message in the UI explaining that now the field has changed, the search needs to be executed again 2) a convenient [search] button is shown so one doesn't have to click into the query field and press enter to trigger the search again.

12:32 pm

Jim Procter

should be Console.warn

should be Console.warn

12:31 pm

Jim Procter

Need to add logic that either 1) modifies config if an existing value of UNIPROT_DOMAIN is found that is legacy.uniprot.org or 2) uses UNIPROT_2022_DOMAIN as a config value so old versions of Jalvi...

Need to add logic that either 1) modifies config if an existing value of UNIPROT_DOMAIN is found that is legacy.uniprot.org or 2) uses UNIPROT_2022_DOMAIN as a config value so old versions of Jalview still work (up until legacy.uniprot.org goes away).

11:38 am

Jim Procter

shouldn't this be a warn ?

shouldn't this be a warn ?

11:36 am

Jim Procter

this comment block should really be after the imports ?

this comment block should really be after the imports ?

11:35 am

Jim Procter

makes sense. I do wonder if there needs to be some indication that any results currently shown are stale, however ?

makes sense. I do wonder if there needs to be some indication that any results currently shown are stale, however ?

11:33 am

Jim Procter

JAL-4036: Jalview 2.11.2.2 and earlier no longer compatible with Uniprot REST API
JAL-4036: Jalview 2.11.2.2 and earlier no longer compatible with Uniprot REST API
Wednesday 30 Mar
4:23 pm

Jim Procter

4:22 pm

Jim Procter

make AlignFrame the parent of FeatureEditor dialog, and have parent Component request focus after...
make AlignFrame the parent of FeatureEditor dialog, and have parent Component request focus after...
4:20 pm

Jim Procter

JAL-3975 forced re-focus to alignframe after Create Sequence Feature dialog.
JAL-3975 forced re-focus to alignframe after Create Sequence Feature dialog.
Monday 07 Mar
8:03 pm

Jim Procter

Thursday 03 Mar
9:26 am

Ben Soares

9:26 am

Ben Soares

Looks good. I've bumped the version to 1.8.3-1.2.12 and recompiled jars.

Looks good. I've bumped the version to 1.8.3-1.2.12 and recompiled jars.

Wednesday 02 Mar
12:20 pm

Jim Procter

JAL-3691 patch toUpper/toLower to use Locale.ROOT for 2.11.2 src
JAL-3691 patch toUpper/toLower to use Locale.ROOT for 2.11.2 src
Friday 11 Feb
4:12 pm

Ben Soares

That looks good. I was almost going to do something similar (I was thinking jalview.log.Log, but jalview.bin.Console is just as good) so happy with this!

That looks good. I was almost going to do something similar (I was thinking jalview.log.Log, but jalview.bin.Console is just as good) so happy with this!

3:49 pm

Ben Soares

Added those in. I'm still converting to String with getCanonicalName in the getLogger(Class ...) method because, e.g. JLogger jlogger1 = JLogger.getLogger(Jalview.class); JLogger jlogger2 = JLogger...

Added those in. I'm still converting to String with getCanonicalName in the getLogger(Class ...) method because, e.g.
JLogger jlogger1 = JLogger.getLogger(Jalview.class);
JLogger jlogger2 = JLogger.getLogger("jalview.bin.Jalview");
should really return an existing logger in the registry if it exists, and the String name is the registry key.

3:40 pm

Ben Soares

I was thinking that JLogger did this comparison anyway, but I've followed it through and it didn't. So I've added if (loglevel.compareTo(this.level) < 0) { return false; } to JLogger.println so t...

I was thinking that JLogger did this comparison anyway, but I've followed it through and it didn't. So I've added

if (loglevel.compareTo(this.level) < 0) { return false; }

to JLogger.println so that a comparison is made between the (enums) of JLogger.level and the message LogLevel, and if it's not going to be logged then it doesn't call the actual logger at all (in this case Log4j, so the log4j object is never even touched [even with logger.isDebugEnabled() because the answer should be no]).
This now applies to all log calls.
The check whether Cache.log != null is equivalent to Cache.log.loggerExists() which already happens within JLoggerLog4j.
There is admittedly the wrapper of the JLogger getting brought to forefront of memory, but that's presumably smaller than the old Cache.log which was a Log4j.Logger object.

2:15 pm

Jim Procter

2:09 pm

Jim Procter

AWESOME JOB !!!! one tiny niggle I had was that Cache.debug/Cache.error doesn't read nicely, so I've abstracted the logging stuff even farther to a new jalview.bin.Console class: see https://source...

AWESOME JOB !!!! one tiny niggle I had was that Cache.debug/Cache.error doesn't read nicely, so I've abstracted the logging stuff even farther to a new jalview.bin.Console class: see https://source.jalview.org/crucible/changelog/~br=update%402fJAL-3949_jalview_bin_Console_to_cure_sanity_even_more/jalview/

1:49 pm

Jim Procter

the reason for conditionals is that Java won't perform the string append operation if logLevel>debug. It saves a minor amount of processing time which is important for code that is executed lots.

the reason for conditionals is that Java won't perform the string append operation if logLevel>debug. It saves a minor amount of processing time which is important for code that is executed lots.

1:46 pm

Jim Procter

You could have .getLogger(Class ..) rather than having to call getCanonicalName ?

You could have .getLogger(Class ..) rather than having to call getCanonicalName ?

1:36 pm

Jim Procter

JAL-3949 An attempt at converting to Log4j 2 -- no output achieved!
JAL-3949 An attempt at converting to Log4j 2 -- no output achieved!
3:32 am

Jim Procter

Thursday 10 Feb
3:08 pm

Jim Procter

3:07 pm

Jim Procter

JAL-3553: Create working getdown appbase and jvl file(s) for each old version of Jalview
JAL-3553: Create working getdown appbase and jvl file(s) for each old version of Jalview
12:12 pm

Jim Procter

Tuesday 08 Feb
11:42 am

Jim Procter

I'm abandoning this review for now (see my comments on the issue). For what its worth since this is a string literal you should at least follow convention JALVIEW_LOGGER rather than a generic 'LOGG...

I'm abandoning this review for now (see my comments on the issue). For what its worth since this is a string literal you should at least follow convention JALVIEW_LOGGER rather than a generic 'LOGGER_NAME'. The code for routing logged content from other loggers should probably be a method on Cache... or even better, separate all this out into the jalview.util.Log4j class so Cache doesn't get polluted with log configuration cruft.

11:24 am

Ben Soares

At the moment it is only used in jalview.bin.Cache, but it is used in separate places where appenders are added. Previous to this it was relying on the String name of the appender being correctly u...

At the moment it is only used in jalview.bin.Cache, but it is used in separate places where appenders are added. Previous to this it was relying on the String name of the appender being correctly used. However the Strings that were being used were not the same so it was broken (I don't know how that manifested itself, but it definitely would not have worked the way it used to!).
Feasibly a separate logger made in the future (e.g. API logger?) will want to add this Appender and will likely not be made in jalview.bin.Cache, so putting a constant String somewhere generically connected with Log4j seemed sensible. Correct it needs documenting!