ibabiankou commented on pull request #144: URL: https://github.com/apache/maven-resolver/pull/144#issuecomment-1018406507
@caiwei-ebay I'd say the PR is ready for review from my perspective. Below are the suggested title and description for the PR. WDYT? `Collect dependencies in breadth-first order` ``` This PR changes dependency processing order from depth-first to breadth-first as a preparation for further improvements (MRESOLVER-228, MRESOLVER-7). **Key changes** 1. NodeStack.find moved to DefaultDependencyCycle since the NodeStack is replaced with a List in the DependencyProcessingContext. 2. Args.dependencyProcessingQueue contains a DependencyProcessingContext for each dependency that should be collected. Note: This PR does not solve the MRESOLVER-133, though one could expect so from the titles. The main goal of the ticket is to implement early exit when resolving the version range. The ticket should be updated to reflect that. ``` @michael-o @cstamas I was thinking about rewriting the history of changes to make review easier, however, the only split that makes sense to me is 1. Use breadth-first order (add the new context and use it) ~95% of the change 2. Move the `find` method to DefaultDependencyCycle and drop NodeStack And it doesn't seem to make the review much easier 😕 Given that the change is fairly small and the majority of the diff is due to moving the `find` and adding `context.` to access the same things from the new place, do you think it is worth the effort? -- 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]
