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

ci(integrity): ensure that no new duplicates are introduced #18748

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

belgattitude
Copy link
Contributor

What does it do?

Introduce a check for duplicates in the CI. this is suggestion after #18745 is merged.

Why is it needed?

Will ensure no new duplicates are introduced over time (often by tools like dependabot...)

How to test it?

N/A

Related issue(s)/PR(s)

Future

Use renovatebot instead of dependabot

While yarn 3+ is very good at keeping duplicates low (pnpm 8+ too), the yarn.lock is often changed by bots (renovate, dependabot...). In my experience renovate is probably the best to use as it's able to dedupe the update PR's. An example of renovate.json5 could be

{
  "extends": ["config:base"],
  "enabled": true,
  "enabledManagers": ["npm", "docker-compose", "dockerfile", "github-actions"],
  "postUpdateOptions": [
    // https://docs.renovatebot.com/configuration-options/#postupdateoptions
    // Will run yarn dedupe --strategy highest
    'yarnDedupeHighest'
  ],
  // ... Kept short for brevity

- uses: actions/setup-node@v4
with:
node-version: 20.x
- name: Monorepo install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While yarn dedupe does not "require" a prior install (it will do it for you), good tip is to pre-install to keep the benefit of yarn-nm-install (more than 2x faster thx to caching)

- name: Monorepo install
uses: ./.github/actions/yarn-nm-install
- name: 👬🏽 Check for duplicated deps
run: yarn dedupe --check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will display the list and fail if there is duplicates

Copy link
Member

Choose a reason for hiding this comment

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

what do you foresee as the process for individuals to solve this check? run the command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my experience yarn 3+ already dedupes very well.

Contributors that would add/remove/update a dependency in their pull requests won’t never trigger a ci check error here.

But if it does fail: ie the contributor edited the lock manually or because of a yarn issue, he will have to run the command: yarn dedupe to generate a new lock file then push it.

There’s sense to add an howto in the contributor guideline. But it will be rare.

in the other hand tools that directly modifies the lock might trigger a failure: Dependabot..,

if it does, someone will need to add a dedupe commit.

hope it clarifies

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a question you can probably answer :)

If dependabot tends to cause dupes, what do you feel would be the best strategy for using it? Is there a way to trick it into behaving better? or will someone see the error and have to know they need to manually pull the PR and dedupe it before merging?

I like this idea but we'll need to set some sort of guide/policy for "when you see this failed test on dependabot, you need to take these specific steps" so that people don't get lost and confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. My personal journey into that was to move away from dependabot (and in general whatever directly change the lock file). Not sure about dependabot recent evolutions, but still seems

dependabot/dependabot-core#5830

There’s ways to circumvent that directly in the ci, cause we can detect the bot user and dedupe but it’s weird at the end.

I’d recommend to try and use renovate instead. You’ll find a note in the issue description. Both tools are great but renovate will dedupe

otherwise yes documentation or add a comment in the ci step to direct the people to an how-to

Another option is to not check for duplicate at all in the ci. Running a scheduled action every week that just dedupe and create a pr if something was deduplicated is very good too. Also possible just run it before releases… the idea is just to prevent them to accumulate without noticing

@joshuaellis joshuaellis changed the base branch from main to develop November 16, 2023 15:16
@joshuaellis joshuaellis added source: tooling Source is GitHub tooling/tests/ect pr: chore This PR contains chore tasks (cleanups, configs, tooling...) labels Nov 16, 2023
@joshuaellis joshuaellis self-assigned this Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community pr: chore This PR contains chore tasks (cleanups, configs, tooling...) source: tooling Source is GitHub tooling/tests/ect
Projects
Status: To be reviewed (Open)
Development

Successfully merging this pull request may close these issues.

None yet

3 participants