kriegaex edited a comment on pull request #83:
URL: https://github.com/apache/maven-shade-plugin/pull/83#issuecomment-873338678


   @JanMosigItemis, maybe you want to reconsider your opinion and simplify the 
PR the way I suggested. I guess, the smaller the change, the less nested the 
control structures, the fewer new imports you pull in without any obvious 
benefit, the higher the chances that someone merges this quickly. You may also 
want to minimise your test change and factor out the actual test refactoring 
(if necessary at all) into either a new PR or at least into a separate commit, 
so we can clearly differentiate the new test case from the refactoring. It 
helps nobody if the PR is just sitting here, rotting.
   
   Having said that, I do not really understand why nobody with the right to 
actually merge this thing has collaborated with you in order to get this off 
the table for so long. I would, if I could, but I am a user and contributor, 
just like you. I have no privileges here. I suggested this fix in January 
already, so I guess the maintainers must be super busy and this plugin does not 
have a high priority for anyone. This surprises me to some extent, because I 
believe that tens of thousands of developers - I want to avoid the hyperbole of 
saying "millions" -  probably use Maven Shade.
   
   **Update:** I wanted to mention one more thing:
   
   > I once was taught to avoid `continue` (or `break` for that matter) in 
loops bc it makes the execution path harder to be traced.
   
   I disagree. It helps avoid nesting and also helps getting exceptional 
conditions out of the way before applying the "happy path" logic. When I see 
`break` or `continue`, I immediately know that I do not have to read on when 
trying to understand or trace in my mind the execution path of a method. I can 
either scroll to the end of the control structure (`break`) or back to the 
beginning of it (`continue`) and simulate the next iteration in my mind. 
Furthermore, in this case the style of using `continue` is already used 4 more 
times in the very same method, so changing it for this one case is like 
applying two paradigms to the same piece of code.


-- 
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-unsubscr...@maven.apache.org

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


Reply via email to