-
Notifications
You must be signed in to change notification settings - Fork 359
Generate Screenshots in CI #715
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
dba8d54
Ignore Ruby debugger history
lcreid ac65d73
Commit screenshots
lcreid e4f9d01
Changed in CI
invalid-email-address 31b988b
Revert "Changed in CI"
lcreid 861011c
Try to avoid double CI run when screenshots change
lcreid 6eea73e
[skip ci] Changed in CI
invalid-email-address c822974
Only run screenshot commit on failure
lcreid d0f47d6
Remove comment
lcreid 609886e
Be safe with CI
lcreid d96e663
Clarify workflow for screenshot changes in main
lcreid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,36 @@ You may find the [demo application](#the-demo-application) useful for developmen | |
| - If your PR fixes an issues, be sure to put "Fixes #nnn" in the description of the PR (where `nnn` is the issue number). Github will automatically close the issue when the PR is merged. | ||
| - When the PR is submitted, check if GitHub Actions ran all the tests successfully, and didn't raise any issues. | ||
|
|
||
| When you create or update a pull request, GitHub automatically runs tests that generate the screenshots in the [`README.md`](/README.md). If any of the screenshots change, GitHub will add an additional commit with the updated screenshots. | ||
|
|
||
| Normally, the screenshots should _not_ change. If the screenshots changed, please review them _carefully_. Some clear reasons why you would want to keep the changed screenshots: | ||
|
|
||
| - Your PR was fixing behaviour that was wrong in the screenshot. | ||
| - You added new examples in the documentation, so there are new screenshots. | ||
| - A change to the images used by GitHub in their actions changes the behaviour of Chrome, although if you think it's this you should probably prepare a separate PR that _only_ updates the screenshots, so it's clear what the change is and why we're making the change. | ||
|
|
||
| Unless you have one of the above reasons, or you have a good explanation for why the screenshots have changed with your PR, you need to get rid of the changed screenshots (i.e. revert the last commit) and fix your PR so the screenshots don't change. (The reason you should revert the commit with the screenshots is so that the next time you push, GitHub will compare against the original screenshots, not the ones changed by your previous push.) Here's how to get rid of the changed screenshots: | ||
|
|
||
| ```bash | ||
| git pull # to bring the additional commit to your local branch. | ||
| git revert HEAD # to remove the changes. | ||
| ``` | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The workflow probably needs to differentiate between master commits and branches for pull requests.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really good point! Thanks! |
||
| Then fix the code and push your branch again. | ||
|
|
||
| If the change was intended, a comment in the PR explaing why the change is expected would be very much appreciated. More than appreciated. It will avoid us having to ask you for an explanation. | ||
|
|
||
| You can run the tests that generate the screenshots locally, but unless your environment is very much like the GitHub CI environment -- Ubuntu running Chrome with default fonts -- all the screenshots will be reported as having changed. To generate the screenshots: | ||
|
|
||
| ```bash | ||
| cd demo | ||
| bundle exec rails test:all | ||
| ``` | ||
|
|
||
| The [Docker development environment](#using-docker-compose) appears to generate screenshots that are the same as what GitHub generates. | ||
|
|
||
| Finally, maintainers may sometimes push changes directly to `main` or use other workflows to update the code. If pushing to `main` generates a commit for screenshot changes, please consider reverting your change immediately by executing the above `pull` and `revert` and another `push`, for the sanity of users who are using the edge (`main` branch) version of the gem. At any rate, review the changes promptly and use your judgement. | ||
|
|
||
| ### 7. Done | ||
|
|
||
| Somebody will shortly review your pull request and if everything is good, it will be | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file modified
BIN
-35 Bytes
(100%)
demo/doc/screenshots/bootstrap/index/00_horizontal_form.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-1.84 KB
(95%)
demo/doc/screenshots/bootstrap/index/01_with_validation_error.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-572 Bytes
(98%)
demo/doc/screenshots/bootstrap/index/03_simple_action_text_example.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-552 Bytes
(98%)
demo/doc/screenshots/bootstrap/index/04_floating_labels.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| desc <<~DESC | ||
| Commit changed files, if any. Meant to be used in CI to commit automatically created files | ||
| (e.g. screenshots). | ||
| DESC | ||
| task :commit do # rubocop:disable Rails/RakeEnvironment | ||
| msg = <<~MSG | ||
| Changed in CI | ||
| Please review the changes in the files in this commit | ||
| carefully, as they were automatically generated during CI. | ||
| Run `git pull` to bring the changes into your local branch. | ||
| Then, if you do not want the changes, run `git revert HEAD`. | ||
| MSG | ||
| system("git config user.name github-actions") | ||
| system("git config user.email github-actions@github.com") | ||
| system("git diff --exit-code -s || git commit --all -m '#{msg}' && git push") | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will automatically commit all changed files in the demo directory after a failure? That does not seem right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't think of a better way to communicate the changed screenshots back into a place where the dev could review the screenshots and see if their code needed to be fixed. GitHub's diff for the screenshots is actually pretty slick. I admit that the revert step may be a nuisance, or people may forget it. But we should still catch changes when reviewing the PR.
I could make the workflow more specific, so we only save the screenshot changes when it was the screenshot diff that failed.
I'm open for other suggestions about how to generate and capture the screenshots when we create new ones, or the change is intended.