Wdio Percy Support v8#11865
Conversation
|
Nice PR, left some minor feedback 👍 |
|
@erwinheitzman I have pushed changes as per the comments above. Can you do another round of review? Thanks! |
|
@christian-bromann could you also review this one? I think it's good to have two pair of eyes on this one 🙂 |
|
This is the corresponding PR for WebdriverIO V7 |
christian-bromann
left a comment
There was a problem hiding this comment.
Initial set of comments.
| return false | ||
| } | ||
|
|
||
| // node v0.10 |
There was a problem hiding this comment.
What is this comment about?
There was a problem hiding this comment.
fs.accessSync was introduced post node 0.10+ so added handling as a failsafe
There was a problem hiding this comment.
WebdriverIO defines which Node version it supports and it expects that all interfaces are supported for these versions. I don't see a need to document this in the code. Furthermore, I don't believe anyone uses a Node.js version older than v1.
|
@christian-bromann Resolved the above comments, can you do another round of review post these changes? Thanks! |
|
Looks good from the BrowserStack side. |
This reverts commit 0ba8af2. "removed # prefix for private fields"
review changes
christian-bromann
left a comment
There was a problem hiding this comment.
Another set of suggestions
|
@christian-bromann I have addressed all the comments, can you please check if it looks good |
|
@rev-doshi happy to merge once the unit test pass |
|
@christian-bromann thank you! I have fixed the UT issue and run them at my end. |
|
Sorry, merged to quickly. I wanted to say: LGTM 👍 thank you for the contribution! |
Proposed changes
BrowserStack Percy Support to capture screenshots & snapshots on pre-existing test builds and generate reports on Percy.
Types of changes
Checklist
Further comments
Reviewers: @webdriverio/project-committers