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

test/lint: Always fetch the target branch if it cannot be found. #12741

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

markylaing
Copy link
Contributor

Previously this was only performed when the GITHUB_BEFORE environment variable was set.

test/lint/golangci.sh Outdated Show resolved Hide resolved
Previously this was only performed when the GITHUB_BEFORE environment
variable was set.

Signed-off-by: Mark Laing <[email protected]>
@markylaing markylaing force-pushed the fetch-target-branch branch 3 times, most recently from bc5b8e4 to d4028d0 Compare January 18, 2024 15:45
@markylaing
Copy link
Contributor Author

@tomponline this seems to be working now. I've changed the script to abort if it doesn't find a revision to check against, otherwise no lint errors will be raised.

I am quite confused as to how we haven't come across this before. It's possible that we just didn't notice the warning and nothing has been linted with golangci for quite some time 😢

@markylaing
Copy link
Contributor Author

@tomponline this seems to be working now. I've changed the script to abort if it doesn't find a revision to check against, otherwise no lint errors will be raised.

I am quite confused as to how we haven't come across this before. It's possible that we just didn't notice the warning and nothing has been linted with golangci for quite some time 😢

Indeed this is from the next PR that changed any Go code (#12649), it was in December: https://github.com/canonical/lxd/actions/runs/7164995944/job/19506095032#step:14:19

@tomponline
Copy link
Member

I'm glad I opportunistically noticed and mentioned it then :)

@tomponline tomponline merged commit 80b9e52 into canonical:main Jan 18, 2024
25 checks passed
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