Shift common installer fields to manifest root level #409
Conversation
mdanish-kh
requested review from
yao-msft and
ryfu-msft
and removed request for
a team
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Took me some time to get around how the tests are added. While I was adding the test case I realized that the properties that are partial classes were not getting compared correctly and we needed to add the .Equals() override for those properties. Also had to redundantly move the methods to autonomous and interactive update methods so that the results are visible in test cases. Updated existing test cases to match the new behavior introduced by this PR. |
src/WingetCreateTests/WingetCreateTests/WingetCreateTests.csproj
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Let me know if I messed up somewhere while addressing the review comments 😄VS is warning me to implement GetHashCode() overrides and separate each partial class into a separate file. Let me know if this is something I should look into or something that we can ignore for now. |
|
/azp run |
|
No commit pushedDate could be found for PR 409 in repo microsoft/winget-create |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
We want to make sure these build warnings go away as I think these may cause issues when we try to publish a new release. Don't worry about the GetHashCode() warnings, it is fine as is.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Edit after merge: - Items at root level are not modified #211
This change modifies the Update flow (both interactive and autonomous) to shift the manifest root values to installer level at the start of the update for easier program logic and then shifting the common installer values back to manifest root at the end of the update for better human readable manifests. Similar shifting done at the end of New Command flow as well.
Broke
PromptPropertiesAndDisplayManifestsinto multiple functions for better separation of concerns and so that shifting can be done before the call to DisplayManifestPreview()Manually verified with packages like DevHome, Terminal, PowerToys, WinGetCreate to make sure I wasn’t breaking any existing CIs with this change.
Also refactored previous function for consistent naming
Edit after merge: This should also fix #211 since the root values are now moved and overwritten to the installer level at the start of the update flow which will make it so that the ProductCode always correctly gets updated and then moved to the root level if it's common between all installers.