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

Update vdiffr svgs after descent changes #2998

Merged
merged 1 commit into from
Nov 13, 2018

Conversation

dpseidel
Copy link
Collaborator

The changes to descent in #2996 broke numerous vdiffr tests by slightly altering plot height (see screenshots below). This PR updates the svg files.

@thomasp85, I've gone through and inspected the mismatches, all containing the same slight shift, but perhaps you should review to confirm this shift is an expected and acceptable change in plot behavior before we update the test case files.

See slight shift in the xaxis height:
vdiffr before/after 2996

The differences are much more pronounced in the "theme-large" tests, e.g. :
theme-large vdiffr before/after 2996

@thomasp85
Copy link
Member

For some reason I didn't see any broken tests locally, which surprised me, so thanks for picking that up

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

All of this is within the bounds of acceptable change. Fixing the alignment of text is bound to cause some slight changes

@dpseidel
Copy link
Collaborator Author

Great, thanks! if your local vdiffr version is not up to date, visual tests will be skipped silently so I'd check that first.

@dpseidel dpseidel merged commit 391c350 into tidyverse:master Nov 13, 2018
@lock
Copy link

lock bot commented May 12, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators May 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants