-
Notifications
You must be signed in to change notification settings - Fork 118
Zizmor fixes #756
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
Draft
weiji14
wants to merge
6
commits into
development
Choose a base branch
from
zizmor-fixes
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Zizmor fixes #756
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
50c8738
Fix dependabot-cooldown
weiji14 07914d0
Fix artipacked
weiji14 305ece3
Fix unpinned-uses
weiji14 42192b9
Fix excessive-permissions
weiji14 c44c0f2
Small change on unit_test.yml
weiji14 4dcca1c
Change to place binder badge only when a /binder comment is made in a PR
weiji14 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,40 @@ | ||
| # Add Binder Link for testing a branch when a PR comment with `/binder` is made | ||
| # https://mybinder.readthedocs.io/en/latest/howto/gh-actions-badges.html#example-2-comment-with-a-binder-badge-in-response-to-a-comment | ||
| name: AddBinderBadge | ||
| on: | ||
| pull_request_target: | ||
| on: [issue_comment] # issues and PRs are equivalent in terms of comments for the GitHub API | ||
|
|
||
| permissions: {} | ||
|
|
||
| jobs: | ||
| badge: | ||
| runs-on: ubuntu-latest | ||
| trigger-chatops: | ||
| # Make sure the comment is on a PR, and contains the command "/binder" | ||
| if: (github.event.issue.pull_request != null) && contains(github.event.comment.body, '/binder') | ||
| runs-on: ubuntu-slim | ||
| permissions: | ||
| pull-requests: write | ||
|
|
||
| steps: | ||
| - uses: manics/action-binderbadge@main | ||
| # Use the GitHub API to: | ||
| # (1) Get the branch name of the PR that has been commented on with "/binder" | ||
| # (2) make a comment on the PR with the binder badge | ||
| - name: comment on PR with Binder link | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | ||
| with: | ||
| githubToken: ${{ secrets.GITHUB_TOKEN }} | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| // Get the branch name | ||
| github.rest.pulls.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: context.payload.issue.number | ||
| }).then( (pr) => { | ||
|
|
||
| // use the branch name to make a comment on the PR with a Binder badge | ||
| var BRANCH_NAME = pr.data.head.ref | ||
| github.rest.issues.createComment({ | ||
| issue_number: context.payload.issue.number, | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| body: `[](https://mybinder.org/v2/gh/${context.repo.owner}/${context.repo.repo}/${BRANCH_NAME}) :point_left: Launch a binder notebook on this branch` | ||
| }) | ||
| }) |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
workflow_runtrigger is apparently dangerous, but also I'm not too sure how to avoid it 🙃. I think the key is that we want the unit tests to be re-ran after the UML diagram commit, need to think of how it can be done more safely.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we need to run the unit tests after the UML diagram commit is because the unit tests passing are required branch protection checks for merging. Is there a better approach to getting the UML diagrams updated that makes it so we don't need this workflow trigger to rerun the unit tests? Otherwise I'm also not sure how to avoid this trigger.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so reading your previous comment at #618 (comment) which links to https://github.com/EndBug/add-and-commit/tree/v10.0.0#the-commit-from-the-action-is-not-triggering-ci, they say:
So I could make sure the commit happens with a Personal Access Token (PAT) instead of the default GITHUB_TOKEN, and then we should be able to remove the
workflow_runtrigger. I had a look around and there seems to be one already used by the traffic action here:icepyx/.github/workflows/traffic_action.yml
Lines 27 to 28 in 31e208a
So we could probably just re-use that token (if that's ok, or we can create another one). How does that sound?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, if deciding to create a new token, I'd recommend adding the secret PAT to a dedicated environment (see: creating an enviroment) so that it can only be accessed in a restricted way. Downside is that a maintainer will need to manually approve the UML workflow run (in addition to approving the PR itself). Up to you on what your risk tolerance is (we'll actually be fairly safe already after this PR's changes).