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.

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

(GH-981) Suggest Get-FileHash when applicable #1692

Conversation

SeanKilleen
Copy link
Contributor

@SeanKilleen SeanKilleen commented Dec 16, 2018

Resolves #981.

For compatibility reasons, Chocolatey suggests using choco install checksum to verify checksums.

But, in Powershell v4 and beyond, users could choose to run Get-FileHash as well.

This change adds a message in applicable powershell versions (v4.x and later) to suggest this course of action, per the up-for-grabs issue.

Work Breakdown & Remaining

✅ Add version check using $PSVersionTable.PSVersion.Major
✅ Add Debug text
❓ Verify that the text is correct when outputted
✅ Build passes
❓ Tests
✅ Update the commit message to wrap at 72 chars (I think I was over in a few places)

Merge latest into my repo.
@SeanKilleen
Copy link
Contributor Author

Hey all,

I believe this change has been successful but I'm a little lacking on the knowledge / context to appropriately test this function (willing to do so if someone can point me in the right direction). In the meantime, I'll read up on the docs.

…nd above

For compatibility reasons, Chocolatey suggests using `choco install
checksum` to verify checksums.

But, in Powershell v4 and beyond, users could choose to run
`Get-FileHash` as well.

This change adds a message in applicable powershell versions (v4.x and
later) to suggest this course of action.
@SeanKilleen SeanKilleen force-pushed the gh-981_checksum-tool-suggestion branch from 898dc2c to c2ec501 Compare December 16, 2018 03:57
@SeanKilleen
Copy link
Contributor Author

Updated commit message to wrap at or before 72 characters.

@ferventcoder
Copy link
Member

Hi @SeanKilleen - thanks, this looks great. However there are two commits here. One is a merge. That means you might have missed https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#contributing-process. Can you remove that commit please? Thanks!

@SeanKilleen
Copy link
Contributor Author

@ferventcoder weird, not sure why that was -- definitely wasn't the intention. I must have opened the PR and then done an update to my fork and merged it in. I'll fix it up. 👍

@SeanKilleen SeanKilleen force-pushed the gh-981_checksum-tool-suggestion branch from b16089a to c2ec501 Compare January 17, 2019 00:36
@ferventcoder
Copy link
Member

Still showing two commits. Not sure why, but I can cherry pick this commit in.

@ferventcoder
Copy link
Member

Validated that this is CRLF line endings for authenticode signing. 👍

@ferventcoder
Copy link
Member

Cherry picked into stable at 8f4fe74. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants