[opencms-dev] How to code-review

[Tobias Herrmann]

Hi Alberto,

you are right, the inclusion of the mavenLocal repository is not 
necessary for the opencms-core build. We do need it in projects like 
alkacon-oamp to be able to build against an unreleased OpenCms version.

The dist_* tasks that build the module ZIPs no longer depend on the 
corresponding jar_* tasks, as they no longer contain the JAR. This has 
been changed in version 10 to improve the setup process. The JARs are 
included in the opencms.war below WEB-INF/lib, importing them with the 
modules during the setup was causing issues with the class-loader.

Regarding the IDE support, please let me know, what the requirements are 
for integration into IDEA or Netbeans. We are using eclipse and have no 
issues.

The gradle build script is very long and complicated. This is mostly due 
to the OpenCms project layout, which has been designed many years ago 
before gradle existed. The most complicated part being the dynamic 
configuration of the module tasks. If you have any ideas, how to improve 
this part, you are very welcome.

Kind regards,

Tobias

-------------------

Tobias Herrmann

Alkacon Software GmbH & Co. KG  - The OpenCms Experts
http://www.alkacon.com - http://www.opencms.org
Am 20.06.2017 um 21:02 schrieb Alberto Gallardo:
> Hi Tobias,
> 
>     under which circumstances do you end up with wrong binaries?
> 
> 
> At least, there are two details that can lead to unexpected results:
> - The inclusion of mavenLocal() as repository: if we build and install 
> any of the maven dependencies locally, these binaries will be used for 
> the gradle build without warnings. We probably want gradle to only build 
> against official artifacts, and not against possible remainders in our 
> .m2/repository.
> - 'dist_*' tasks: they do not depend on the corresponding 'jar_*' task. 
> If the binaries are on their paths, Gradle builds these zips without 
> rebuilding them, even if it were necessary.
> 
>     Also, the test cases do take time to execute, this is not related to
>     any possible inefficiencies of the build script. You don't need to
>     run the test cases to build the binaries.
> 
> 
> That's very true. And I haven't found any room for performance 
> improvements yet. (I still wanted to take a look into parallel 
> executions, but this has no priority.)
> 
> 
>     Other tasks that take a lot of time are generating GWT resources,
>     this is a general issue with GWT and not related to the build script.
> 
>     And if you build the JavaDoc jars this will take a lot of time as
>     well, but this is related to the amount of source code and not to
>     the build script.
> 
> 
> The point is that the build script is missing some javadoc files (see 
> http://www.opencms-wiki.org/wiki/Talk:OpenCms_Documentation/Server_installation/OpenCms_shell). 
> And is very hard to notice, because it issues a lot of warnings.
> 
>     So, if you think there is a way to significantly safe time during
>     build, please point out, what you think should be changed.
> 
> 
> I don't have yet any 'silver bullet' to get 'significant' time savings, 
> but I still have some hope:
> 
> - With gradle 4.0, the build cache is production ready, and I was hoping 
> to be able to use it. I have to further investigate this. (Again, it'd 
> be great to use the gradle wrapper)
> - To improve incremental builds, the 'enhance' task should also be 
> reviewed. Apparently, it gets always executed (the 
> 'outputs.upToDateSpec' trick doesn't seem to work). And it changes also 
> the classes, so it invalidates caches, and after invoking it, the 
> compileJava will be triggered again. This could be a problem of the 
> gradle version used: that's the reason I'd like to have the wrapper, but 
> I don't really know until I test a bit more.
> 
> My biggest issues though are related to the bad IDE support of some 
> functionalities that are in theory natively supported. For example, the 
> org.opencms.* source sets define different java.includes with the same 
> base moduleSrcFolder. IDEA doesn't like this, and I'm not sure if it 
> could have side effects. IDEA also has native support for vaadin, gwt 
> and some other extended features. But not with the current script, or 
> not without configuring the project (I don't know how). And for that, I 
> have to first analyse the script and find how to tame the beast.
> 
> The gradle script is pretty big and hard to grasp. It has taken me quite 
> a while to understand it (and I'm still on it). And while on reviewing 
> it, I though 'why not try to improve it?'. That's why I also tried to 
> deduplicate code and get rid of the warnings.
> 
> But as you pointed out, all of this could be seen as 'cosmetic changes'. 
> I didn't want to distract you from other tasks until I myself am 
> satisfied with refactorings and changes.
> 
> 
> 
> Regards,
> 
> 
> Alberto
> 
> 
> _______________________________________________
> This mail is sent to you from the opencms-dev mailing list
> To change your list options, or to unsubscribe from the list, please visit
> http://lists.opencms.org/cgi-bin/mailman/listinfo/opencms-dev
> 
> 
>