[GitHub] [maven] AlexanderAshitkin edited a comment on pull request #526: [MNG-7129] Incremental build and shared cache feature

[GitBox <gi...@apache.org>]

AlexanderAshitkin edited a comment on pull request #526:
URL: https://github.com/apache/maven/pull/526#issuecomment-903311772


   Hi
   Thanks a lot for the feedback.  The idea of this PR is not to finalize the featurem but  to demonstrate curent state, and gather feedback. Realistically looking at the current state, there is an expectedly long way forward to align this feature with all the guidelines and architecure requirements. Though the feedback is reasonable and this is exactly what we are looked for, the implementation in this branch is stable and better to be kept as is. Reworks better to be done atop of that in a smaller incremental steps. To move forward following apporach appears reasonable:
   1) Merge this to a feature branch as is as a baseline
   2) Find way to safely enable for those who is interested under some "Experimental feature" umbrella (with minimal modifications) and merge it in an official distribution (of course safely and compliantly)
   3) Gather and accumulate community feedback to produce a sort of feature roadmap. The ultimate target is integrating it to an official release with all the requirements satisified. 
   4) Continue work in feature branch to adress p.2 decisions and achive mutual agreement between community members
   5) integrate it in an agreed/safe way and remove "Experimental flag"
   
   
   Small clarifications to review comments:
   1) @rmannibucau rework repositories - absolutely reasonable, should be aligned. There are some open question to discuss, like best layout for cache, associated cases - like purging cache, store cache metadata, etc. All this could evolve to repositories design discussion. 
   2) @rmannibucau Replace with built-in dependencies - reasonable, to be discussed. On use of jaxb - the reasone to use it was schema validation. If modello supports that - could easily be reused.  ENCODING_DETECTOR - it supports wide set ofencodings beyond UTF unlike of AwareInputStreamReader. So to be discussed case by case, but in overall agree here.
   4) @rfscholte on splitting PR - though technically of course possible, but decomposing seems to be a lot of work by itself. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be tageted to master or to another feature branch.
   5) regarding location of the feature - it feels like it should be a seprate module or an extension. Happy to rework in that direction - any guidance is very welcome
   6) @hboutemy - we will restore original formatting in core and similar to not add noise in review
   
   From my perspective there are folowing implementation related streams of work:
   * Move feature to a separate module/extension safely isolated from core and find a proper way to plug it in safely.
   * Align implementation with built-in dependencies (like the mentioned bandwagon, modello, etc)
   * Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage
   
   Thank you


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org