-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update branch and baseline docs #325
Conversation
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.
Left some questions—nothing blocking—and made some small typo/nitpicky changes. But this is so much clearer and more helpful than it was.
@@ -121,7 +118,7 @@ You can see the ancestor builds listed on the build page: | |||
|
|||
![Ancestor Builds](../../images/ancestor-builds.png) | |||
|
|||
#### Calculating a snapshot baseline from the ancestor build(s) | |||
#### Calculate a snapshot baseline from the ancestor build(s) | |||
|
|||
Once we’ve got the ancestor builds for a build, the algorithm to calculate the baseline for any given snapshot goes like this: |
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 may not be for this PR—this isn't part of your changes—but I wonder if this description of the baseline algorithm should be an (un)ordered list or otherwise grouped together semantically and/or visually?
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.
Also, in the first line below, should “viewport combination” be “mode” instead?
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 updated the copy to "mode".
Tell me more about the unordered list/grouping? If it's easy let's do it here.
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'm going to merge this to expedite the updates. Let's do a follow up PR if we want to change this section (assign me as a reviewer!).
https://linear.app/chromaui/issue/DX-789/docs-simplify-branches-and-baselines-page
Edit the baseline docs for coherence. I changed the perspective of the doc from baselines (a UI Tests centric pov) to how UI Tests and UI Review work with snapshots.
The copy is up to date, but I need to add one image.