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

Update codeql.yml #32125

Merged
merged 4 commits into from
Nov 24, 2020
Merged

Update codeql.yml #32125

merged 4 commits into from
Nov 24, 2020

Conversation

XhmikosR
Copy link
Member

Specify the branches

I'm seeing a weird behavior, i.e. fixes when there's no change in a branch. It's like CodeQL is mixing up the branches. I'm not sure if this will fix the issue, but it's what they suggest in their workflow example

@jhutchings1 if you have any suggestions let me know :)

Specify the branches
@jhutchings1
Copy link
Contributor

@XhmikosR Can you point me to a pull request where things got messed up? We've been chasing a couple of issues with our alert diffing functionality, and I'm wondering if what you're seeing is an instance of that.

cc: @rneatherway @tibbes

@XhmikosR
Copy link
Member Author

@jhutchings1 will do as soon as I hit it again. It seems to happen if we push or make PRs sort of at the same time to different branches.

So, you think this PR shouldn't land? I mostly made it so that we follow the upstream action more closely and to see if that would fix the issue.

@XhmikosR
Copy link
Member Author

@jhutchings1 https://github.com/twbs/bootstrap/runs/1400254580 This is a v4-dev branch and the "fix" refers to the main branch.

@jhutchings1
Copy link
Contributor

Aha, yeah, I think you're hitting one of the race conditions in our diffing functionality. We're working on it, but haven't got it all fixed just yet. The changes you proposed in the PR are absolutely fine, but they won't fix the root cause, unfortunately. Hoping to have these issues fixed up soon!

@XhmikosR
Copy link
Member Author

Ah, perfect, I knew we couldn't be the only ones hitting this :)

I will simplify the workflow since we only need one language and I'll wait monitor for any fixes. Thanks for the reply!

@XhmikosR XhmikosR marked this pull request as ready for review November 17, 2020 20:42
@rneatherway
Copy link

@jhutchings1 https://github.com/twbs/bootstrap/runs/1400254580 This is a v4-dev branch and the "fix" refers to the main branch.

@XhmikosR 👋 I'm the engineer working on this issue. I had a look at this specific example and what you're seeing here is some older compatibility code we have for workflows that do analysis on: pull_request. This code looks for a pull request for an analysed commit and if one isn't found it speculatively does the comparison against the default branch. If you then opened a pull request against the default branch everything would appear correct. We are phasing this behaviour out. Your change to the workflow restricts the on: push triggers and should therefore prevent you seeing this in the future for that branch.

@XhmikosR XhmikosR merged commit d61f506 into main Nov 24, 2020
@XhmikosR XhmikosR deleted the XhmikosR-patch-2 branch November 24, 2020 06:35
XhmikosR added a commit that referenced this pull request Nov 24, 2020
Specify the branches and clean up the comments
XhmikosR added a commit that referenced this pull request Nov 24, 2020
Specify the branches and clean up the comments
@XhmikosR XhmikosR added the CI label Nov 24, 2020
@XhmikosR XhmikosR added skip-changelog So that the release drafter action doesn't include it build and removed skip-changelog So that the release drafter action doesn't include it labels Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants