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 feature but to demonstrate curent state, and to gather initial feedback to decide on the further plan. 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 in small increments looks easier to move forward. At the moment 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). Ideally merge it to an official distribution (of course safely and compliantly) under a feature flag which will provide a sort of user consent. 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 use cases - like purging cache, store cache metadata, etc. All this could evolve to some repositories design decisions. Overall approach was is to minimize changes in existing classes. 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 and will leave feature in a half state. Our idea is to merge this as is to a feature branch and continue with smaller enhancements from there as necessary. They could be targeted 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 - guidance here 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 Wagon, Modello, etc) * Agree proper approach to tests implementation for this feature (like introducing wiremock?) and achieve necessary level of confidence in tests coverage * Work on supplementary features like versionless projects/builds and version injection, better plugins paramters api and some other 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
