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

docs(readme): Add new related project, lighthouse-github-reporter #6307

Merged
merged 4 commits into from
Oct 23, 2018

Conversation

carlesnunez
Copy link
Contributor

Summary
This PR ads a new related project into Lighthouse called Lighthouse-github-reporter. More info on the issue.
Related Issues/PRs
#6097

@googlebot
Copy link

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. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@carlesnunez
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@carlesnunez carlesnunez changed the title Update readme.md docs(readme): Add new related project, lighthouse-github-reporter Oct 17, 2018
readme.md Outdated
@@ -276,6 +276,8 @@ This section details services that have integrated Lighthouse data. If you're wo
## Related Projects
Other awesome open source projects that use Lighthouse.

* **[lighthouse-gh-reporter](https://github.com/addyosmani/webpack-lighthouse-plugin)** - give Lighthouse-CI the power of put reports as a comment on your pull requests.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe your url is wrong. It's the same url as webpack-lighthouse-plugin

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

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

A few nitpicks.

readme.md Outdated
@@ -276,6 +276,8 @@ This section details services that have integrated Lighthouse data. If you're wo
## Related Projects
Other awesome open source projects that use Lighthouse.

* **[lighthouse-gh-reporter](https://github.com/addyosmani/webpack-lighthouse-plugin)** - give Lighthouse-CI the power of put reports as a comment on your pull requests.
Copy link
Member

Choose a reason for hiding this comment

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

Checking the history of this file, we add new integrations to the bottom of the list usually. So I'd move this to right under lighthouse4u.

Copy link
Member

Choose a reason for hiding this comment

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

nit: give Lighthouse-CI the power of put to make reports as a comment on your pull requests in Github.

@carlesnunez
Copy link
Contributor Author

PR Changes done! Sorry for the typo errors and position mistake! 👍

@carlesnunez
Copy link
Contributor Author

Hello! Why is the CI failing exactly? What I did wrong?

@wardpeet
Copy link
Collaborator

It's appveyor, no worries it's pretty flaky

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

LGTM!

@carlesnunez
Copy link
Contributor Author

Thank you for your word fixes @brendankenny ! 😸

@brendankenny
Copy link
Member

@wardpeet if you're good here

Copy link
Collaborator

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

LGTM

@paulirish paulirish merged commit 93686eb into GoogleChrome:master Oct 23, 2018
@carlesnunez carlesnunez deleted the patch-1 branch October 23, 2018 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants