Merging CISLAM Experimental model (no Test Suite available) in JGrassTools#8
Merging CISLAM Experimental model (no Test Suite available) in JGrassTools#8mcfoi wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Hi, can I ask you to remove commented variables? If they are not needed, please just remove them.
There was a problem hiding this comment.
I forgot this mail: now OmsCislam should be free from dead/commented code.
On 3 November 2014 03:50, Andrea Antonello notifications@github.com wrote:
In
hortonmachine/src/main/java/org/jgrasstools/hortonmachine/modules/hydrogeomorphology/cislam/OmsCislam.java:
- */
- // #############################################
- // van Genuchten parameters
- // #############################################
- @description(OMSCISLAM_inAlfaVanGen_DESCRIPTION)
- @Unit("1/m")
- @ui(JGTConstants.FILEIN_UI_HINT)
- @in
- public GridCoverage2D inAlfaVanGen = null;
- /*
- @description(OMSCISLAM_pAlfaVanGen_DESCRIPTION)
- @in
- public double pAlfaVanGen = 6.5;
- */
- @description(OMSCISLAM_inNVanGen_DESCRIPTION)
Hi, can I ask you to remove commented variables? If they are not needed,
please just remove them.—
Reply to this email directly or view it on GitHub
https://github.com/moovida/jgrasstools/pull/8/files#r19717772.
|
@mcfoi , I gave a first quick look at everything and commented inline as you can see. One thing that is wrong more or less everywhere, is the use of: In the Oms* modules, there is no need for that, since they are not loaded in the GUI. And environments that want to generate one for GridCoverages and FeatureCollections do not need that. This UI annotation has been introduced only when using file paths, since in that case it is string and we never know if it is a path or something different. So check the inputs and outputs. Remove them from the Oms* classes and add it to the modules classes. |
|
I fixed everything else beside the issued I commented back. Marco On 3 November 2014 04:16, Andrea Antonello notifications@github.com wrote:
|
The wrapper of the main module is working correctly. All Oms sub-modules are fine. Wrappers for sub-modules might not be fully functional.