[Meta] Workflow to catch/flag unintended `yarn.lock` updates in PRs

joshuarrrr commented 2 years ago

Is your feature request related to a problem? Please describe.

It's common for developers to unintentionally update their local yarn.lock file while developing locally (simply bootstrapping the project may install slightly different patch versions of dependencies for example). But for maintaining dependencies, we'd like to make sure any dependency changes are intentional, and considered in isolation for the purpose of safely backporting to the right branches. So, in general feature PRs should not include yarn.lock changes unless the feature actually adds a new dependency (and has a corresponding package.json update).

Describe the solution you'd like

I'd like our PR GitHub actions to include a check for yarn.lock changes with no package.json changes that advises the contributor to remove yarn.lock from the commit (and potentially to create a new PR if necessary).

Describe alternatives you've considered

In the past, I've manually flagged this in reviews, but that's both error-prone and slower than an automated approach.

Additional context

Examples of recent feature PRs with unrelated yarn.lock changes:

"Good" examples of standalone dependency change PRs:

ashwin-pc commented 2 years ago

I like this idea but i'm not sure if we want to make it a workflow. An incorrect lockfile is an indication that the lock file is outdated and should probably be updated.

Instead of a workflow, cant we add it as a git precommit method?

joshuarrrr commented 2 years ago

Sure - but precommit hooks can be annoying to override, in the case where the lockfile does need an update.

ashwin-pc commented 2 years ago

Not really. You simply need to add the flag --no-verify or -n

opensearch-project / OpenSearch-Dashboards

[Meta] Workflow to catch/flag unintended `yarn.lock` updates in PRs #2429