-
Notifications
You must be signed in to change notification settings - Fork 0
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
Move visual-diff-plugin to testing repo #16
Conversation
25b7c80
to
4b4bdb1
Compare
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 is probably sufficient for now and we can add tests once this is actually doing something useful.
const goldenFileName = `${join(dirname(session.testFile), 'screenshots', ciDir, 'golden', browser, dir, newName)}.png`; | ||
const currentFileName = `${join(dirname(session.testFile), 'screenshots', ciDir, 'current', browser, dir, newName)}.png`; |
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.
Doesn't need to be figured out now, but based on the crazy names we've seen in the past hitting Windows limits, wondering if we want to make this shorter. I'm not particularly attached to screenshots
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.
That's a good point -- could be vdiff
?
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.
Yeah that's what I was thinking, although we're already being a bit inconsistent with when we use vdiff
and when we use visual-diff
. Maybe we try to keep all consumer facing pieces vdiff
, like the group name, this folder, etc? Or do we just change everything to vdiff
(like this plugin name)?
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 good with being consistent everywhere with vdiff
.
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.
Merging as-is for now, and will address this probably in the golden migration PR.
🎉 This PR is included in version 0.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Copied from BrightspaceUI/core#3610