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(contributing): add tips for audit and gatherer PRs #10690

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

brendankenny
Copy link
Member

based on the questions in #10662 (comment)

Quick q around the tests, typically for a new audit we have a unit test for the gatherer + audit, update the jest snapshots, then run a yarn update:sample-json? Does the protobuf stuff come into play here, as in would yarn compile-proto && yarn build-proto-roundtrip be needed also? I also see an Updating fixture dumps doc and a yarn update:sample-artifacts command.

I've tried to update CONTRIBUTING a bit to hopefully capture that and removed some outdated advice. As with all docs updates obviously we could have much more here to help contributors, but happy to bikeshed about what's actually here without expanding scope much :)

@umaar it would be great if you get a chance to look at this and had any advice as well.

@brendankenny brendankenny requested a review from a team as a code owner May 1, 2020 18:25
@brendankenny brendankenny requested review from exterkamp and removed request for a team May 1, 2020 18:25
@@ -8,59 +8,40 @@ We tag issues that are good candidates for those new to the code with [`good fir

## Follow the coding style

The `.eslintrc` file defines all. We use [JSDoc](http://usejsdoc.org/) with [TypeScript](https://github.com/Microsoft/TypeScript/wiki/JSDoc-support-in-JavaScript). Annotations are encouraged for all contributions.

## Pull request titles
Copy link
Member Author

Choose a reason for hiding this comment

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

this, CLA, and "Adding Images to a Readme" sections were just moved down the page

Choose a reason for hiding this comment

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

Thanks for help

## Contributing a patch

If you have a contribution for our [documentation](https://web.dev/learn/#lighthouse), please submit it in the [web.dev repo](https://github.com/GoogleChrome/web.dev).

1. Submit an issue describing your proposed change to the repo in question.
Copy link
Member Author

Choose a reason for hiding this comment

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

weirdly generic

dbw_tester.html. To update this file, start a local server on port `8080` and serve the directory `lighthouse-cli/test/fixtures`. Then run:

```sh
npm run start -- --output=json --output-path=lighthouse-core/test/results/sample_v2.json http://localhost:8080/dobetterweb/dbw_tester.html
Copy link
Member Author

Choose a reason for hiding this comment

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

super old

@umaar
Copy link
Contributor

umaar commented May 4, 2020

Cool! Thanks for using that feedback! I think it explains things really well now

@brendankenny brendankenny merged commit bfdd899 into master May 19, 2020
@brendankenny brendankenny deleted the contribs branch May 19, 2020 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants