Making verify package method default to the highest version found #6026
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the VerifyPackage() method to ensure registration of the highest version available package instead of breaking early when a higher version is found. Previously, the method would break out of the loop as soon as it found a package version higher than the target, potentially registering a version lower than the highest available.
Key changes:
- Tracks the highest version found during package iteration instead of breaking early
- Ensures consistent registration to the highest available package version
- Removes conditional insertion logic in favor of always tracking the highest version
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
guimafelipe changed the title 
DrusTheAxe left a comment
DrusTheAxe left a comment There was a problem hiding this comment.
Good find!
Some additional optimization suggestions
Thank you! This PR is still a draft because I did not manage to create tests that reproduce this issue. I would like your help on understanding the reasons here. I wanted to use a test similar to the one I created for the PR #6021. In this PR, I added this test scenario, in which I use a singleton package with I higher version than the usual test one. When I use this test scenario on this current PR, the What is the required scenario for the |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
This PR is a change on the
VerifyPackage()method. This method previously worked iterating over the packages found for a specific package family. In the case of the Singleton package, which is version-less in its package family name, would be all the singleton packages found considering the version tag (experimental, test, etc...). In the case of the main, theMajor.Minorversion is considered in the filtering. The problem is that the iteration was breaking out of the loop as soon as it found a package with a version higher than the one we are verifying. This could make the manager register for a Main package in a version lower than the highest version available. Although it is not breaking, it is inconsistent as we would just default to registering to the first option found.The code was changed to register the highest version found.
A microsoft employee must use /azp run to validate using the pipelines below.
WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.
For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.