doc

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Ah it does run when a column selection is made in a split frame (and propagated to the other panel). Which seems to work! Maybe just needs a less clumsy AlignmentViewport.isColSelChanged() - this m...

Ah it does run when a column selection is made in a split frame (and propagated to the other panel). Which seems to work!
Maybe just needs a less clumsy AlignmentViewport.isColSelChanged() - this method seems to be trying to do two different things.

Suggesting renaming to PROPERTY_COLUMNS = "columns" as this is used to notify changes to column selection or visibility, not sequences

Suggesting renaming to PROPERTY_COLUMNS = "columns" as this is used to notify changes to column selection or visibility, not sequences

I think this code path rarely gets run (AlignmentViewport.notifySequence() rarely gets called). So what is it there for?

I think this code path rarely gets run (AlignmentViewport.notifySequence() rarely gets called).
So what is it there for?

Suggest renaming notifyAlignment() to notifyAlignmentChanged(), and notifySequence() to notifyColumnChange() as it it fired for changes to column (not sequence) selection or visibility.

Suggest renaming notifyAlignment() to notifyAlignmentChanged(), and notifySequence() to notifyColumnChange() as it it fired for changes to column (not sequence) selection or visibility.

Unclear what notifySequence() adds. As placed here, it is hardly ever reached.

Unclear what notifySequence() adds. As placed here, it is hardly ever reached.

Not sure how to review this (and the appletgui/js classes) without pointers to documentation and/or examples of how it is used.

Not sure how to review this (and the appletgui/js classes) without pointers to documentation and/or examples of how it is used.

Needs proper Javadoc

Needs proper Javadoc

Files added to doc folder: are this intended as working documents, or publishable documentation? Bearing in mind that that are findable on the web and a user might reasonably expect them to provide...

Files added to doc folder: are this intended as working documents, or publishable documentation? Bearing in mind that that are findable on the web and a user might reasonably expect them to provide documentation.
e.g. http://source.jalview.org/gitweb/?p=jalview.git;a=blob;f=doc/JalviewJS-startupParams.md;h=9ff33529b2dbc2da34d2a573ccf5d042194fbb0d;hb=refs/heads/Jalview-JS/develop

Due to the number of changes this is likely to be a 'best efforts' review.

Due to the number of changes this is likely to be a 'best efforts' review.

Commits to JS-develop from 2nd to 25th June 2020
Commits to JS-develop from 2nd to 25th June 2020
I suggest we close this. It's all ancient history now.

I suggest we close this. It's all ancient history now.

I have added a large number of commits to this review which were tagged "JAL-3253-applet" or "Jalview-JS/develop-3253" or similar. This should be just the issue id for JIRA to associate the commit ...

I have added a large number of commits to this review which were tagged "JAL-3253-applet" or "Jalview-JS/develop-3253" or similar. This should be just the issue id for JIRA to associate the commit with the issue.

This reset also needs to happen in copyAlignPanel() (which also calls loadViewport() where the flag is set on).

This reset also needs to happen in copyAlignPanel() (which also calls loadViewport() where the flag is set on).

Can't reproduce this now (can't "bring desktop to front"). Will resolve and reopen if it recurs.

Can't reproduce this now (can't "bring desktop to front"). Will resolve and reopen if it recurs.

No proprietary format files please! Is this meant to be a temporary working record or a permanent one. If the latter, can it be in csv or tsv format?

No proprietary format files please! Is this meant to be a temporary working record or a permanent one. If the latter, can it be in csv or tsv format?

No problem, the call to ScoreModels.getInstance() is unchanged, only how it is implemented.

No problem, the call to ScoreModels.getInstance() is unchanged, only how it is implemented.

Deferred to JAL-3265

Deferred to JAL-3265

Failing test getValidSourceDBRefExceptionXTest() (Network not Functional test). Now throws NullPointerException but coded only to expect SiftsException. Traced to uninitialized field DBRefEntry.ucv...

Failing test getValidSourceDBRefExceptionXTest() (Network not Functional test).
Now throws NullPointerException but coded only to expect SiftsException.
Traced to uninitialized field DBRefEntry.ucversion. Initialising this to "" (like version) resolves this.

I am nervous of doing much processing in a static block in case of errors giving ExceptionInInitializerError. Also possible dependency on order of classloading if one class's static codes reference...

I am nervous of doing much processing in a static block in case of errors giving ExceptionInInitializerError.
Also possible dependency on order of classloading if one class's static codes references another's.
Maybe move back to inside constructor?
In fact, was there any need to change this class from the 'standard' singleton pattern, if we don't want it "per applet"?

? there are no tests that call this getInstance() for this class. And if there were they would normally be in the same package so the method could be either package private, or called by reflection.

? there are no tests that call this getInstance() for this class. And if there were they would normally be in the same package so the method could be either package private, or called by reflection.

I am seeing some odd z-plane behaviour, not sure if it is specific to this branch. If the desktop is brought to front, I can't then bring any other panel to front.

I am seeing some odd z-plane behaviour, not sure if it is specific to this branch. If the desktop is brought to front, I can't then bring any other panel to front.

Too much change around ASequenceFetcher, DbRoot etc to be able to review from the diffs. They seem to work when testing! (The code here may now be overcomplicated for the simplified (post-DAS) world.)

Too much change around ASequenceFetcher, DbRoot etc to be able to review from the diffs. They seem to work when testing!
(The code here may now be overcomplicated for the simplified (post-DAS) world.)

So what if anything used to access this? Why I hate public fields!

So what if anything used to access this? Why I hate public fields!

Could write file.contains("/") rather than file.indexOf('/') >= 0, for simplicity and readability. This is buggy - also in the original. Fetching from URL "https://www.ebi.ac.uk/Tools/dbfetch/dbfet...

Could write file.contains("/") rather than file.indexOf('/') >= 0, for simplicity and readability.
This is buggy - also in the original. Fetching from URL "https://www.ebi.ac.uk/Tools/dbfetch/dbfetch/pdb/1a70/pdb" results in file.substring(52, 18).
Issue JAL-3272 raised for that.

I have restored member variables to the top of the class declaration to keep standard source code ordering.

I have restored member variables to the top of the class declaration to keep standard source code ordering.

We have Platform.openURL() calling BrowserLauncher.openURL() and and vice versa, confusing. Suggest we make BrowserLauncher a package private class and route all calls via Platform.

We have Platform.openURL() calling BrowserLauncher.openURL() and and vice versa, confusing.
Suggest we make BrowserLauncher a package private class and route all calls via Platform.

Possibly the most strangely designed class in Jalview, but this version of it seems to work as well as any other!

Possibly the most strangely designed class in Jalview, but this version of it seems to work as well as any other!

This goes back to 2005...but it would be tidier to overload Cache.setProperty() to take a boolean value, and apply Boolean.toString() there...oh well.

This goes back to 2005...but it would be tidier to overload Cache.setProperty() to take a boolean value, and apply Boolean.toString() there...oh well.

Is this still 'TODO'? I just tried it and it worked fine (uniref50.fa + exampleFeatures.txt, popup menu on FER_CAPAA, Link sub-menu, Pfam family target).

Is this still 'TODO'? I just tried it and it worked fine (uniref50.fa + exampleFeatures.txt, popup menu on FER_CAPAA, Link sub-menu, Pfam family target).