Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(#3477) Replace Get-ChecksumValid with Assert-ValidChecksum cmdlet. #3525

Merged
merged 5 commits into from
Jan 28, 2025
Merged

(#3477) Replace Get-ChecksumValid with Assert-ValidChecksum cmdlet. #3525

merged 5 commits into from
Jan 28, 2025

Conversation

vexx32
Copy link
Member

@vexx32 vexx32 commented Oct 3, 2024 •
edited
Loading

Description Of Changes

  • Remove Get-ChecksumValid
  • Add Assert-ValidChecksum
  • Add tests for command to verify behaviour matches the old command.
  • Add alias for backwards compatibility with packages.
  • Minor fixes for the doc-gen script to address issues I ran into.
  • Added a framework we can use in future for deprecating old aliases or to-be-removed commands in a simple and standardised way.

This PR does not contain the compiled XML help information for the command. In discussion with @gep13 we agreed that it would be better to compile that in the final release. I will have a docs PR linked here in a moment, watch this space.

Note: As this PR is part of #3477 and is thus a breaking change to a degree, we will need to cut a support/2.x branch once this is merged, so that none of the breaking changes associated with #3477 inadvertently make their way into the 2.x versions.

Motivation and Context

See #3477

Documentation PR: chocolatey/docs#1075

Testing

Tested locally with the accompanying Pester tests:

  1. From an administrative PowerShell session
  2. Run .\build.ps1
  3. Run .\Invoke-Tests.ps1 -Tag AssertValidChecksum
  4. From VS, run the integration tests and verify there are no unexpected issues.

Proving out the deprecated commands framework

After running the above, close and reopen the admin PowerShell session, and then follow these steps:

  1. Apply the following patch: Get-ValidCheckSum-deprecation-verification.patch
  2. Rerun the prior steps to run the build and tests again, including the code and test change.
  3. Monitor the output from the tests; near the start of the test run, you should see a warning issued about the test calling Get-ChecksumValid and that the alias is deprecated and should be replaced with Assert-ValidChecksum

Operating Systems Testing

Windows 10

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v3 compatibility checked?

Related Issue

Part of #3477

@vexx32 vexx32 requested review from gep13 and corbob October 3, 2024 20:38
@vexx32 vexx32 marked this pull request as draft October 3, 2024 20:40
@vexx32
Copy link
Member Author

vexx32 commented Oct 3, 2024

There is one final matter to resolve before this PR is fully ready; help documentation for the cmdlet. I will have a corresponding PR opened on the docs repository by tomorrow, and then update the Chocolatey.PowerShell.dll-help.xml file for this PR accordingly.

@vexx32 vexx32 marked this pull request as ready for review October 4, 2024 17:53
@vexx32 vexx32 force-pushed the assert-checksum branch 2 times, most recently from 1627d8f to 7574df1 Compare October 4, 2024 18:28
This was referenced Oct 4, 2024
Draft
Open
@vexx32
Copy link
Member Author

vexx32 commented Oct 7, 2024

There is one final matter to resolve before this PR is fully ready; help documentation for the cmdlet. I will have a corresponding PR opened on the docs repository by tomorrow, and then update the Chocolatey.PowerShell.dll-help.xml file for this PR accordingly.

After talking with @gep13 we have agreed it is best not to update the XML here at present, as it would require increasingly complicated messing around with the associated docs PRs as we rewrite more commands, and will instead opt to update the help XML file before a release artifact is created.

@vexx32 vexx32 force-pushed the assert-checksum branch 3 times, most recently from a1983e4 to f9be806 Compare October 15, 2024 14:44
gep13
gep13 requested changes Dec 5, 2024
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you get this PR rebased onto the head of develop?

src/Chocolatey.PowerShell/Shared/ChocolateyCmdlet.cs Show resolved Hide resolved
@vexx32 vexx32 changed the base branch from develop to feature/cmdlets December 20, 2024 16:31
vexx32 added 5 commits January 27, 2025 15:03
@vexx32
(maint) Add tags for each PowerShell Cmdlet test
b7d2c81 
As a small convenience, to allow us to run tests for an invididual
cmdlet when doing development testing.
@vexx32
(chocolatey#3477) Add Assert-ValidChecksum cmdlet
70d4648 
This command replaces the Get-ChecksumValid helper function, adding an
alias for compatibility purposes.
@vexx32
(chocolatey#3477) Add tests for Assert-ValidChecksum
6a5cd3a 
Add tests to ensure the behaviour of rewritten command matches the
original behaviour.
@vexx32
(maint) Update documentation generation script
02fdac5 
When working on this, some irregularities were noted with the generation
of the documentation, so these changes avoid the issues that arose.
@vexx32
(maint) Add command deprecation framework
69c0cac 
This allows us to maintain a simple list of deprecated aliases and the
new command name, or commands that are being removed, and automatically
warn when that command is called.
@vexx32
Copy link
Member Author

vexx32 commented Jan 27, 2025

@gep13 are we good to merge this? just gave it a rebase so I think everything's more or less sorted?

gep13
gep13 approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gep13
Copy link
Member

gep13 commented Jan 28, 2025

@vexx32 said...
are we good to merge this?

Yes, I think we can move forward with this! Thanks for getting this done!

@gep13 gep13 merged commit 4aa8444 into chocolatey:feature/cmdlets Jan 28, 2025
5 checks passed
@vexx32 vexx32 deleted the assert-checksum branch February 14, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gep13 gep13 gep13 approved these changes

@corbob corbob Awaiting requested review from corbob

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vexx32 @gep13