nsoft commented on PR #2017: URL: https://github.com/apache/shiro/pull/2017#issuecomment-2701721954
I understand the need to choose, but I'm not understanding your criteria. You speak of "repeating" the work in the PR. What is the point of folks going to the effort of creating a PR if not to save you work?. The criteria I would suggest is which PR passes more tests, which PR's changes look more in line with the existing attempts to control dependencies. Also I don't know how explicit the focus on a given EE level is in the other PR, but if it's spring boot based approach, I worry that it's going to be EE10 oriented and that won't mesh with guice and other libraries that are not EE10 supportive yet. If there is anything you don't understand about what I've done I'll be happy to answer, but I don't understand your desire to throw out BOTH pull requests and create a third (or maybe I'm misunderstanding your intent). I've been contributing to various Apache projects since [2002](https://bz.apache.org/bugzilla/show_bug.cgi?id=1550#c14) (Apache Ant) and I'm a committer since 2019 (Lucene/Solr). I've never heard of such a thing as the starting point, except when a committer already has a set of changes ready to go. I've of course seen PR's abandoned after an attempt to make them work, but I don't understand what you mean by "repeating" them. If there is something I've done wrong or if you need me to rebase it to your 3.x branch that I missed I can help on that if you like. I spent almost a week developing this to the point where it passes all but a dozen+ tests which AFAIK only fail because of classpath/ or swallowed exceptions during test harness startup. I decided to submit here because I knew I was swimming against the learning curve of tools I don't know well, but I'm sure with some more effort I can get the remaining tests passing. I just figured there was a chance someone who knew the tool would know how to do things like get arquillian to write out logs where I can find it since maven seems to swallow anything relating to the plugin/tool startup. Often I had to debug into things to find out what was actually failing, and that of course continually runs afoul of IDE holding onto references to jars that aren't actually used anymore (many IDE caches, and pom imports died to bring us this information ;) ) I definitely wouldn't advocate the merging of this until all tests pass, and I'm currently using the result to develop against locally and will report if it shows any problems. The vast majority of the patch is just s/javax/jakarta/ replacements so reading it should take a lot less time than the number of files would normally imply. The most mysterious thing is likely to be the deletion of spring remoting related classes and tests, and that's because spring remoting doesn't exist in newer versions. It had been deprecated and was finally dropped. Also, I dropped the guice3 and guice4 testing and focused on the guice 7 as simply "guice" (I didn't think there would be desire to maintain ancient classpaths for running guice 3, 4, 5, 6, and 7 tests). If the other patch is a better place to start from, because it passes more tests or touches fewer poms, or you like the libraries it selected better that's fine. I won't be at all concerned if you use an better PR than mine. I just don't understand this notion that you need to repeat and compare. Such a thing would leave you with 3 way variations to sort out: 1. Tool vagaries (some of which the other PR probably dealt with), 2. Submitted PR variances (some of which are likely developer choice, not tool related), 3. and things that still need to be done. It should be simpler to trust your own tests, trust one PR or the other (with review) and fix-up the remaining tests. It's not getting released instantly so there's time to find and fix issues that remain and/or are discovered. -- 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]
