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 dependabot CI #101

Merged
merged 4 commits into from
Jul 17, 2023
Merged

Update dependabot CI #101

merged 4 commits into from
Jul 17, 2023

Conversation

emdash-ie
Copy link
Contributor

Don’t use root working directory

This was breaking the dependabot runs, because “/” was interpreted here as the root directory of the runner, not the top level of the repository, and there wasn’t permission to create files and directories there. Probably this worked at first and only broke recently.

Use same workflow for dependabot and pulls

These had diverged a bit, and I think it’d be good for most things to be shared (everything that dependabot can run), by using if conditions on the individual steps rather than the whole job.

Inline scripts into workflow

This makes it easier to skip steps for dependabot PRs, and possibly makes better use of the UI.

This was breaking the dependabot runs, because “/” was interpreted here
as the root directory of the runner, not the top level of the
repository, and there wasn’t permission to create files and directories
there. Probably this worked at first and only broke recently.
…even if someone else pushed the specific commit.
@emdash-ie emdash-ie requested a review from a team July 17, 2023 14:22
Comment on lines 36 to 41
- name: Dump GitHub context
id: github_context_step
run: echo $JSON
env:
JSON: ${{ toJSON(github) }}

Choose a reason for hiding this comment

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

Would this potentially expose any sensitive info? Was this mainly for debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think any sensitive info is masked, for example in this build expanding the step shows that the token is masked.

This isn’t something I added, but I did have the same doubts about it. Let’s get rid of it – it’s useful for understanding the structure of the context and checking values, but we can always add the step back for debugging a given execution if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done now

These had diverged a bit, and I think it’d be good for most things to be
shared (everything that dependabot _can_ run), by using if conditions on
the individual steps rather than the whole job.
This makes it easier to skip steps for dependabot PRs, and possibly
makes better use of the UI.
Copy link

@marialani marialani left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@emdash-ie emdash-ie merged commit 4606706 into main Jul 17, 2023
@emdash-ie emdash-ie deleted the fix-dependabot-ci branch July 17, 2023 15:26
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.

2 participants