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

Create codeql-analysis.yml #5611

Merged
merged 2 commits into from Oct 2, 2020
Merged

Create codeql-analysis.yml #5611

merged 2 commits into from Oct 2, 2020

Conversation

sigmavirus24
Copy link
Contributor

No description provided.

Copy link
Member

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

Interesting service! Some of the comments in the yml are a little much, up to you if you want to prune them down a bit.

Also, if this makes security vulnerabilities public have we tried running it locally and fixed issues before making this automated?

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved

on:
push:
branches: [master, proposed/3.0.0]
Copy link
Member

Choose a reason for hiding this comment

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

Is there value in checking the v3 branch if it's likely not going to be released?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better question: Is there value in keeping it around if it's not going to be released?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it in the second commit as well

Copy link
Member

Choose a reason for hiding this comment

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

I think there's a lot of valuable work on the proposed/3.0.0 branch that could be released at some point. I was keeping it up to date until we had some of the crazier commit history changes in 2018. I think we can get it back in sync but it's going to take a pretty tedious merge. Alternatively, we could hand pick the relevant feature commits, but that's also a big time investment.

That said, I don't think there's any value in checking the current proposal branch here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Frankly, I think there's a lot of internal refactoring that could make a 3.0 easier to release and maintain long-term. It's something I've thought about for over a year

@sigmavirus24
Copy link
Contributor Author

To all of your questions: I don't know. I saw the announcement and thought it wouldn't hurt here. I'll look more closely later

Remove proposed/3.0.0 branch, only ever run against Python sans matrix option
Copy link
Member

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM

@sigmavirus24 sigmavirus24 merged commit 4f6c018 into master Oct 2, 2020
@sigmavirus24 sigmavirus24 deleted the add-codescanning branch October 2, 2020 22:10
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants