-
Notifications
You must be signed in to change notification settings - Fork 574
Conversation
adieuadieu
commented
Aug 2, 2017
•
edited
Loading
edited
- rename getHtml() to html()
- fix some linting errors
- manually run prettier (cuz we're momentarily ghetto like that.)
- tweak to yarn.lock
🔥 |
@@ -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) |
There was a problem hiding this comment.
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 👍
There was a problem hiding this comment.
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.