Skip to content

Add composite workflow action for checking user permissions prior to updating snapshots - #268

Merged
brichet merged 6 commits into
jupyterlab:mainfrom
martinRenou:check_perm
Jan 14, 2026
Merged

Add composite workflow action for checking user permissions prior to updating snapshots#268
brichet merged 6 commits into
jupyterlab:mainfrom
martinRenou:check_perm

Conversation

@martinRenou

@martinRenou martinRenou commented Jan 13, 2026

Copy link
Copy Markdown
Member

Comment thread .github/actions/update-snapshots-check-permission/action.yml Outdated
Comment thread .github/actions/update-snapshots-check-permission/action.yml Outdated
martinRenou and others added 2 commits January 13, 2026 16:27
Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com>
@martinRenou

Copy link
Copy Markdown
Member Author

Thanks @brichet !

I've been able to confirm the reusable worfklow works as expected from that PR

@mfisher87

Copy link
Copy Markdown
Member

This is technically a "composite action", not a "reusable workflow" 😝 (sorry, I just expected something different based on the PR title when Martin and I were chatting on a call this morning!)

https://docs.github.com/en/actions/concepts/workflows-and-actions/reusing-workflow-configurations#reusable-workflows-versus-composite-actions

@brichet brichet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @martinRenou, LGTM.

@martinRenou martinRenou changed the title Add reusable workflow for checking user permissions prior to updating snapshots Add composite workflow action for checking user permissions prior to updating snapshots Jan 14, 2026
@martinRenou

Copy link
Copy Markdown
Member Author

Thanks a lot for the review and suggestion @brichet ! Let's hold before merging, @krassowski had some suggestion in jupyterlab/jupyterlab#18334, looking into it now.

@mfisher87 thanks! I renamed the PR

@martinRenou

Copy link
Copy Markdown
Member Author

Done and checked again that it works.

pushed_at="$(echo "$pr" | jq -r .pushed_at)"

if [[ $(date -d "$pushed_at" +%s) -gt $(date -d "$COMMENT_AT" +%s) ]]; then
echo "Updating is not allowed because the PR was pushed to (at $pushed_at) after the triggering comment was issued (at $COMMENT_AT)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe for a follow up PR, but I wonder if we could add a comment or replace the previous 👍 with a 👎, to have a visual indication that something did not work. Same for the step below.

Otherwise it looks good, thanks.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Makes sense! Yes a comment saying the bot failed, with a link to the logs, would be great. Plus, the link to the logs could probably always show up in a comment, so that users know where to look for.

Agreed to make that as a follow-up 👍🏽

@brichet

brichet commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Thanks @martinRenou.

This was referenced Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants