Skip to content
This repository has been archived by the owner on Nov 24, 2018. It is now read-only.

Prepare for v1.1 release #143

Merged
merged 9 commits into from
Aug 2, 2017
Merged

Prepare for v1.1 release #143

merged 9 commits into from
Aug 2, 2017

Conversation

adieuadieu
Copy link
Collaborator

@adieuadieu adieuadieu commented Aug 2, 2017

  • rename getHtml() to html()
  • fix some linting errors
  • manually run prettier (cuz we're momentarily ghetto like that.)
  • tweak to yarn.lock

@adieuadieu adieuadieu requested a review from joelgriffith August 2, 2017 15:56
@joelgriffith
Copy link
Contributor

🔥 :shipit:

@@ -159,7 +159,7 @@ const chromeless = new Chromeless({
- [`exists(selector: string)`](docs/api.md#api-exists)
- [`screenshot()`](docs/api.md#api-screenshot)
- [`pdf(options?: PdfOptions)`](docs/api.md#api-pdf)
- [`getHtml()`](docs/api.md#api-gethtml)
- [`html()`](docs/api.md#api-html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, I was actually going back and forth about the naming of getHtml() vs html() when I implemented this.

I originally called it getHtml() to avoid confusion with setHtml(), but I actually prefer html(), as it's more in line with the naming of screenshot() and pdf().

I like this change 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@seangransee glad that you like it! The maintainers had a conversation about API design goals yesterday where we decided on this naming convention. I'll share more about the design goals soon in a docs/api-design.md linked to via CONTRIBUTING.md.

@adieuadieu adieuadieu merged commit bb8009e into schickling:master Aug 2, 2017
@seangransee seangransee mentioned this pull request Aug 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants