Github Action to check deprecations in PR #19666
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
miagilepner
requested changes
Mar 22, 2023
peteski22
approved these changes
Mar 24, 2023
marcboudreau
approved these changes
Mar 24, 2023
| uses: actions/checkout@24cb9080177205b6e8c946b17badbe402adc938f # v3.4.0 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| fetch-depth: 0 # by default the checkout action doesn't checkout all branches |
There was a problem hiding this comment.
fetch-depth: 0 means getting the complete repository history. Is that necessary for this job? Alternatively, removing the fetch-depth parameter will result of a depth of 1.
There was a problem hiding this comment.
I think we need fetch-depth: 0 as we check the deprecations on changes when compared to origin/main branch. When I remove this, I get "fatal: Not a valid object name origin/main" and it shows all the deprecations in the code. Basically we need the origin/main branch to get the diff in the current branch.
kubawi
approved these changes
Mar 24, 2023
miagilepner
approved these changes
Mar 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://hashicorp.atlassian.net/browse/VAULT-8439
Recommendation:
This PR uses the
staticcheckcmdline tool to identify the use of deprecated functions.This was initially attempted with
golangci-linttool withstaticcheck. It produced inconsistent results, and there are many related issues on GitHub. Furthermore, the author ofstaticcheckis unaware of its inclusion ingolangci-lint. Therefore, I used thestaticchecktool as it is more reliable.The recommendation in the ticket was to create a GitHub action that includes an allowlist for certain findings, but this would prevent us from identifying if deprecated functions are utilized in new code.
To address this issue, the check runs on all files included in the PR.
staticcheckruns at package level, so we need to grep the details for the changes that are included in the PR and display it. It usesrevgrepto do this.To check deprecations locally using the script, follow these steps:
./scipts/deprecations-checker.sh <optional branch name to compare with>on vault/vault-enterprise repository path or any package pathFor example:
./scripts/deprecations-checker.sh main./scripts/deprecations-checker.sh(Running this from vault repo path)
If no branch name is specified, the command will show all the deprecations in the code.
GHA runs this wrt the PR's base ref branch.
This will be added to developer documentation after this PR is approved. Also, this is not a required check.