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 all dependencies using Dependabot #613

Merged
merged 5 commits into from
Jan 18, 2024
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 20, 2023

This PR enables Dependabot updates for npm and GitHub Actions including:

  1. Checks run at monthly intervals
  2. Checks for unstaged changes after build
  3. Grouped PRs for "build", "test" and "tooling" dependencies

Previously only security-related updates were included

Probably wait for #612 to merge first

@colinrotherham colinrotherham added the dependencies Pull requests that update a dependency file label Nov 20, 2023
@colinrotherham colinrotherham self-assigned this Dec 4, 2023
@romaricpascal romaricpascal added this to the Next milestone Jan 11, 2024
@36degrees
Copy link
Contributor

I'm not sure if it's realistic for us to stay on top of weekly Dependabot PRs – have added an agenda item to our next 'dev catch up' to discuss what we want to do.

@colinrotherham colinrotherham requested a review from a team as a code owner January 18, 2024 16:17
.github/dependabot.yml Outdated Show resolved Hide resolved
.github/dependabot.yml Outdated Show resolved Hide resolved
package.json Outdated
@@ -20,6 +20,7 @@
"build:css": "csso src/autocomplete.css -o dist/accessible-autocomplete.min.css",
"build:js": "cross-env NODE_ENV=production webpack --progress",
"build": "run-s \"build:js\" \"build:css\"",
"postbuild": "node scripts/check-staged.mjs",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want this to run whenever we run npm run build locally? I'm not sure I follow the thinking 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.

Yeah think so, for local development you'll run npm run dev instead

From our contributor PRs you'll see everyone misses it currently and follows up with a commit

Output will also stay silent unless dist/ has changed

Copy link
Contributor

Choose a reason for hiding this comment

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

From our contributor PRs you'll see everyone misses it currently and follows up with a commit

Won't that still happen, if contributors are using npm run dev instead of npm run build?

Where are we expecting npm run build to be run where we want this to happen? Are there any places in CI where we're not adding --ignore-scripts?

I won't block this PR on it, but unless there's a clear reason for this change I'd be tempted to remove this change and have the 'Check for unstaged changes' job call node scripts/check-staged.mjs directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't that still happen, if contributors are using npm run dev instead of npm run build?

Nah we use webpack-dev-server so output isn't written to disk for npm run dev

We use npm run build for the "To build the project for distribution" docs + commit hook

I'm happy either way so I'll make your changes, not a problem

* Group ‘build’ for dependencies that might modify dist
* Group ‘test’ for dependencies that might break checks
* Group ‘tools’ for all other dependencies
@colinrotherham colinrotherham merged commit b6b0c55 into main Jan 18, 2024
3 checks passed
@colinrotherham colinrotherham deleted the dependabot-all branch January 18, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants