docs(CONTRIBUTING): update contributing.md #1973
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CONTRIBUTING.md
Outdated
| ## API guidelines | ||
|
|
||
| When authoring new API methods, consider the following: | ||
| - expose as little as needed. When in doubt, don't expose |
There was a problem hiding this comment.
expose as little information as needed. When in doubt, don’t expose new information.
| ``` | ||
| npm run unit -- --filter=waitFor |
There was a problem hiding this comment.
We can't. Though it's trivial to implement if we ever need it back.
ebidel
reviewed
Feb 6, 2018
| - 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` | ||
| - all string literals must be small case. This includes event names and option values |
There was a problem hiding this comment.
There have been questions in slack about adding new sugar apis (page.exist()). It might be nice to call out a bullet that discusses our philosophy on adding new apis.
There was a problem hiding this comment.
@ebidel good point; added a note
JoelEinbinder
approved these changes
Feb 7, 2018
ebidel
approved these changes
Feb 8, 2018
drew-diamantoukos
pushed a commit
to drew-diamantoukos/puppeteer
that referenced
this pull request
Feb 9, 2018
- Adding missing language tags to markdown code blocks. - Switching `npm` to `yarn` in README to be consistent with what repo is using locally/with travis. Removed unneeded and for yarn command per code review Also removing from travis and appveyor yarn invocations Fixed merge conflicts and yarn references Removed yarn.lock and added language to code fence Cheeky capital letter Touched up numbering (and also switched git email account due to CLA mis-match) feat(Page): add `setCacheEnabled(enabled)` to Page object (puppeteer#1609) This change adds in the `Page.setCacheEnabled(enabled)` method to toggle ignoring cache for each request. Fixes puppeteer#1556. docs(CONTRIBUTING): update contributing.md (puppeteer#1973)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.