search

janus-idp / backstage-showcase

This repo is moving to https://github.com/redhat-developer/red-hat-developer-hub
https://showcase.janus-idp.io
Apache License 2.0
113 stars 149 forks source link

Fail the build if the yarn.lock file is changed after running yarn install #949

Closed gashcrumb closed 7 months ago

gashcrumb commented 9 months ago

What do you want to improve?

Often when running yarn install after updating the yarn.lock file contains changes.

What is the current behavior?

This is not ideal as it means there's inconsistencies between what folks are committing and what is being tested in github actions.

What will the new behavior be?

The build action will fail if there's changes to the yarn.lock file after running yarn install

See this comment for more information:

Yes this is a copy of https://github.com/janus-idp/backstage-plugins/issues/1169

nickboldt commented 7 months ago

See https://issues.redhat.com//browse/RHIDP-1204 for updates.

PRs for this:

PR is failing already with

After 'yarn install', workspace is dirty! The following files have changed:

app-config.example.yaml
app-config.yaml
yarn.lock

-- https://github.com/janus-idp/backstage-showcase/actions/runs/8646895996/job/23707183425?pr=1174

and 

After 'yarn install', workspace is dirty! The following files have changed:

app-config.example.yaml
app-config.yaml

-- https://github.com/janus-idp/backstage-showcase/actions/runs/8646912247/job/23707235821?pr=1175

Should we exclude app-config.* from the list of files that can be dirty during a release?

gashcrumb commented 7 months ago

I think not to be honest. For me it's a red flag that there's something going on that needs to be investigated. Or it indicates that an upgrade has migrated the configuration files, in which case I think that this kind of change to the files should be required to be included in a PR update as well.

nickboldt commented 7 months ago

so you're saying this is a GOOD and EXPECTED failure, and the change proposed by this PR is approved?

gashcrumb commented 7 months ago

yep :smile:

gashcrumb commented 7 months ago

I take that back after looking at the PR action :smile: . Turns out the PR check build is copying the app config into place for some reason, so we should leave those two changed files as-is for now and just focus on the yarn.lock file.

nickboldt commented 7 months ago

PRs amended to only complain if a yarn.lock is changed.

Can I get a :+1: on them now?

nickboldt commented 7 months ago

Woo! Snyk PRs are now failing with

After 'yarn install', workspace is dirty! The following files have changed:

app-config.example.yaml
app-config.yaml
yarn.lock

https://github.com/janus-idp/backstage-showcase/actions/runs/8661941105/job/23752905502?pr=1104

So... I'm going to resolve this.