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.

Sign up for GitHub

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

Jump to bottom

docs(CONTRIBUTING.md): fix some typos #4917

Merged
merged 1 commit into from Sep 9, 2019
Merged

docs(CONTRIBUTING.md): fix some typos #4917

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 25 additions & 22 deletions CONTRIBUTING.md
View file
Open in desktop
Expand Up @@ -12,8 +12,8 @@
* [Public API Coverage](#public-api-coverage)
* [Debugging Puppeteer](#debugging-puppeteer)
- [For Project Maintainers](#for-project-maintainers)
* [Releasing to NPM](#releasing-to-npm)
* [Updating NPM dist tags](#updating-npm-dist-tags)
* [Releasing to npm](#releasing-to-npm)
* [Updating npm dist tags](#updating-npm-dist-tags)
<!-- gen:stop -->

# How to Contribute
Expand Down Expand Up @@ -76,6 +76,7 @@ npm run lint
## API guidelines

When authoring new API methods, consider the following:

- Expose as little information as needed. When in doubt, don’t expose new information.
- Methods are used in favor of getters/setters.
- The only exception is namespaces, e.g. `page.keyboard` and `page.coverage`
Expand Down Expand Up @@ -123,9 +124,9 @@ To deliver to a different location, use "deliver" option:

## Writing Documentation

All public API should have a descriptive entry in the [docs/api.md](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md). There's a [documentation linter](https://github.com/GoogleChrome/puppeteer/tree/master/utils/doclint) which makes sure documentation is aligned with the codebase.
All public API should have a descriptive entry in [`docs/api.md`](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md). There's a [documentation linter](https://github.com/GoogleChrome/puppeteer/tree/master/utils/doclint) which makes sure documentation is aligned with the codebase.

To run documentation linter, use:
To run the documentation linter, use:

```bash
npm run doc
Expand All @@ -147,7 +148,7 @@ A barrier for introducing new installation dependencies is especially high:
- Tests should be *hermetic*. Tests should not depend on external services.
- Tests should work on all three platforms: Mac, Linux and Win. This is especially important for screenshot tests.

Puppeteer tests are located in [test/test.js](https://github.com/GoogleChrome/puppeteer/blob/master/test/test.js)
Puppeteer tests are located in [`test/test.js`](https://github.com/GoogleChrome/puppeteer/blob/master/test/test.js)
and are written with a [TestRunner](https://github.com/GoogleChrome/puppeteer/tree/master/utils/testrunner) framework.
Despite being named 'unit', these are integration tests, making sure public API methods and events work as expected.

Expand Down Expand Up @@ -178,7 +179,7 @@ npm run unit -- --break-on-failure
fit('should work', async function({server, page}) {
const response = await page.goto(server.EMPTY_PAGE);
expect(response.ok).toBe(true);
})
});
```

- To disable a specific test, substitute the `it` with `xit` (mnemonic rule: '*cross it*'):
Expand All @@ -189,7 +190,7 @@ npm run unit -- --break-on-failure
xit('should work', async function({server, page}) {
const response = await page.goto(server.EMPTY_PAGE);
expect(response.ok).toBe(true);
})
});
```

- To run tests in non-headless mode:
Expand Down Expand Up @@ -218,7 +219,7 @@ node --inspect-brk test/test.js

## Public API Coverage

Every public API method or event should be called at least once in tests. To ensure this, there's a coverage command which tracks calls to public API and reports back if some methods/events were not called.
Every public API method or event should be called at least once in tests. To ensure this, there's a `coverage` command which tracks calls to public API and reports back if some methods/events were not called.

Run coverage:

Expand All @@ -232,42 +233,44 @@ See [Debugging Tips](README.md#debugging-tips) in the readme.

# For Project Maintainers

## Releasing to NPM
## Releasing to npm

Releasing to npm consists of the following phases:

Releasing to NPM consists of 3 phases:
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm assuming the number wasn't updated when the fourth phase was added.

1. Source Code: mark a release.
1. Bump `package.json` version following the SEMVER rules and send a PR titled `'chore: mark version vXXX.YYY.ZZZ'` ([example](https://github.com/GoogleChrome/puppeteer/commit/808bf8e5582482a1d849ff22a51e52024810905c)).
Copy link
Member Author

Choose a reason for hiding this comment

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

Drive-by question: are PRs like this created manually? I'm asking because the <!-- GEN:empty-if-release -->-style comments imply otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

These PRs are created manually, but the <!-- GEN:empty-if-release --> are updated with our markdown preprocessor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I'll submit a follow-up CL that explicitly calls out that one needs to run npm run doc after bumping the package.json version number.

2. Make sure the PR passes **all checks**.
- **WHY**: there are linters in place that help to avoid unnecessary errors, e.g. [like this](https://github.com/GoogleChrome/puppeteer/pull/2446)
3. Merge the PR.
4. Once merged, publish release notes using the "create new tag" option.
Copy link
Member Author

Choose a reason for hiding this comment

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

the "create new tag" option

What is this referring to exactly? On https://github.com/GoogleChrome/puppeteer/releases I see a "Draft a new release" button -- is that what is meant?

Is there some tool or git log-based command-line script you're using for the release notes (example)?

Copy link
Member Author

Choose a reason for hiding this comment

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

#4940

- **NOTE**: tag names are prefixed with `'v'`, e.g. for version `1.4.0` tag is `v1.4.0`.
2. Publish `puppeteer` to NPM.
- **NOTE**: tag names are prefixed with `'v'`, e.g. for version `1.4.0` the tag is `v1.4.0`.
2. Publish `puppeteer` to npm.
1. On your local machine, pull from [upstream](https://github.com/GoogleChrome/puppeteer) and make sure the last commit is the one just merged.
2. Run `git status` and make sure there are no untracked files.
- **WHY**: this is to avoid bundling unnecessary files to NPM package
- **WHY**: this is to avoid adding unnecessary files to the npm package.
3. Run [`pkgfiles`](https://www.npmjs.com/package/pkgfiles) to make sure you don't publish anything unnecessary.
4. Run `npm publish`. This will publish `puppeteer` package.
3. Publish `puppeteer-core` to NPM.
1. Run `./utils/prepare_puppeteer_core.js`. The script will change the name inside `package.json` to `puppeteer-core`.
2. Run `npm publish`. This will publish `puppeteer-core` package.
4. Run `npm publish`. This publishes the `puppeteer` package.
3. Publish `puppeteer-core` to npm.
1. Run `./utils/prepare_puppeteer_core.js`. The script changes the name inside `package.json` to `puppeteer-core`.
2. Run `npm publish`. This publishes the `puppeteer-core` package.
3. Run `git reset --hard` to reset the changes to `package.json`.
4. Source Code: mark post-release.
1. Bump `package.json` version to `-post` version and send a PR titled `'chore: bump version to vXXX.YYY.ZZZ-post'` ([example](https://github.com/GoogleChrome/puppeteer/commit/d02440d1eac98028e29f4e1cf55413062a259156))
- **NOTE**: make sure to update the "released APIs" section in the top of `docs/api.md`.
- **NOTE**: no other commits should be landed in-between release commit and bump commit.

## Updating NPM dist tags
## Updating npm dist tags

For both `puppeteer` and `puppeteer-core` we maintain the following npm tags:

For both `puppeteer` and `puppeteer-firefox` we maintain the following NPM Tags:
Copy link
Member Author

Choose a reason for hiding this comment

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

I think puppeteer-firefox was a typo here, and puppeteer-core is what was meant.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks! That's right!

- `chrome-*` tags, e.g. `chrome-75` and so on. These tags match Puppeteer version that corresponds to the `chrome-*` release.
- `chrome-stable` tag. This tag points to the Puppeteer version that works with current Chrome stable.
- `chrome-*` tags, e.g. `chrome-75` and so on. These tags match the Puppeteer version that corresponds to the `chrome-*` release.
- `chrome-stable` tag. This tag points to the Puppeteer version that works with the current Chrome stable release.

These tags are updated on every Puppeteer release.

> **NOTE**: due to Chrome's rolling release, we take [omahaproxy's linux stable version](https://omahaproxy.appspot.com/) as *stable*.

Manging tags 101:
Managing tags 101:

```bash
# list tags
Expand Down