Skip to content

Workflows: Augment workflow to ensure failure with invalid change notes #7946

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

Merged
merged 3 commits into from
Feb 11, 2022

Conversation

aeisenberg
Copy link
Contributor

No description provided.

@aeisenberg aeisenberg force-pushed the aeisenberg/check-change-not branch 3 times, most recently from 62d4585 to ca1317a Compare February 10, 2022 21:48
@aeisenberg
Copy link
Contributor Author

@aeisenberg aeisenberg force-pushed the aeisenberg/check-change-not branch 2 times, most recently from bd01c86 to 142a929 Compare February 10, 2022 21:50
@aeisenberg aeisenberg force-pushed the aeisenberg/check-change-not branch from 142a929 to 9441ea9 Compare February 10, 2022 21:53
Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

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

A couple minor things worth looking at.

@@ -8,6 +8,7 @@ on:
- "*/ql/src/**/*.qll"
- "!**/experimental/**"
- "!ql/**"
- ".github/workflows/check-change-not.yml"
Copy link

Choose a reason for hiding this comment

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

Suggested change
- ".github/workflows/check-change-not.yml"
- ".github/workflows/check-change-note.yml"

@@ -8,6 +8,7 @@ on:
- "*/ql/src/**/*.qll"
- "!**/experimental/**"
- "!ql/**"
- ".github/workflows/check-change-not.yml"

Copy link

Choose a reason for hiding this comment

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

Should change */ql/src/**/*.qll to */ql/**/*.qll (and same for .ql). Otherwise we're not running on changes that only affect the library packs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was committed accidentally. I think we should exclude tests and example packs from here. So, I will add a line like this: */ql/lib/**/*.qll. Same for *.ql. It's not really related to this change, but I'll add it anyway.

github.event.pull_request.draft == false

run: |
codeql pack release --groups cpp,csharp,java,javascript,python,ruby,-examples,-test,-experimental
Copy link

Choose a reason for hiding this comment

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

Does this actually fail the step if codeql returns non-zero? I thought you needed set -e to make that happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is a workflow run that I intentionally failed. https://github.com/github/codeql/runs/5148220083?check_suite_focus=true It fails the job as well.

@aeisenberg
Copy link
Contributor Author

Library pack changes were being ignored.
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

Looks great to me. Thanks for adding this check.

@aeisenberg aeisenberg merged commit 0f3d780 into main Feb 11, 2022
@aeisenberg aeisenberg deleted the aeisenberg/check-change-not branch February 11, 2022 17:25
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.

3 participants