OpenSearch-Dashboards
OpenSearch-Dashboards copied to clipboard
[Meta] Workflow to catch/flag unintended `yarn.lock` updates in PRs
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:
- https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2149#discussion_r982753604
- https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2351/files#diff-51e4f558fae534656963876761c95b83b6ef5da5103c4adef6768219ed76c2de
- https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2384#discussion_r975761849
"Good" examples of standalone dependency change PRs:
- https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2425/files
- https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2415
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?
Sure - but precommit hooks can be annoying to override, in the case where the lockfile does need an update.
Not really. You simply need to add the flag --no-verify or -n