-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
docs(readme): update lighthouse-mocha-example repo and description #9158
Conversation
Older example with mocha doesn't work with current lighthouse version ( older example is based on v1 ). This is the updated example repository link with a working example of lighthouse tests with mocha and chai.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Co-Authored-By: cjamcl <[email protected]>
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@hoten looks like there's some issue with CLA on your account after I accepted the commit suggestion that you added. |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
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.
LGTM
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
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 can see the original hasn't been updated since 2016, but it is a little weird to replace it with a completely different project (though with the same goal) with the same name? Maybe that's ok, though.
cc @justinribeiro if you have an opinion
Yeah, I've been meaning to update the older mocha example to be closer inline with my more robust Jest example (https://github.com/justinribeiro/lighthouse-jest-example). I'm fine with replacing it, though the name thing might throw people (that example and my old one aren't exactly the same concept-wise, but probably splitting hairs). |
@brendankenny @justinribeiro I have no issue with renaming it to something different. Just kept as it is since the name felt relevant to what is in the repository. Edit: Would like suggestions so I'll update it to whatever seems fit. |
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.
@justinribeiro has the shiny new jest one in there now so this update LGTM :)
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.
sure, SG
Summary
Older example with mocha doesn't work with current lighthouse version (older example is based on v1). This is the updated example repository link with a working example of lighthouse tests with mocha and chai.
Is this a bugfix, feature, refactoring, build related change, etc?
This is a documentation related change.
Describe the need for this change
Older example doesn't work since it is based on an older version of lighthouse and is no longer maintained.
Related Issues/PRs
None.