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

treat metadata file differently locally from CI #103

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

dlockhart
Copy link
Member

Follow-up from #94 based on some feedback.

When running locally, we don't want it messing with the .vdiff.json metadata file that was generated by CI and checked into source control. So locally it'll read & write that file from .vdiff/.vdiff.json which is automatically .gitignore'd. It'll only write it locally when goldens are being re-generated.

In CI, it'll reference the metadata file from the root .vdiff.json, and will always write to it. If modified, it'll get committed with the rest of the golden changes.

@dlockhart dlockhart requested a review from a team as a code owner July 26, 2023 18:21
Copy link
Member Author

Choose a reason for hiding this comment

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

Deleting this for now. The first time the action runs, it'll re-create it!

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that, let's us verify it works as expected!

return { name: b.name, version: b.version };
});
writeFileSync(metadataPath, JSON.stringify(metadata, undefined, '\t'));
if (isCI || updateGoldens) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we're ok with the browser versions being slightly off if you're using --filter or --grep with --golden, meaning any old goldens you don't update could've been created by an older browser. There isn't really a way around it without storing the browser info with every png like we'd originally thought about. And it's only an issue locally, not in CI

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, but yeah not worried about it. This stuff is really only useful in CI anyway -- if you're reusing old goldens locally from an old browser... I'm not sure what you're doing. I've almost talked myself into only using this information in CI, period, haha.

@dlockhart dlockhart merged commit 0968f47 into main Jul 26, 2023
1 check passed
@dlockhart dlockhart deleted the dlockhart/fix-metadata branch July 26, 2023 21:41
@ghost
Copy link

ghost commented Jul 26, 2023

🎉 This PR is included in version 0.22.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants