Skip to content

Conversation

@simonmaddox
Copy link

@simonmaddox simonmaddox commented Sep 9, 2021

A copy of the PR on Netatmo#6

See more here, here and here.

@cbaker6
Copy link

cbaker6 commented Nov 12, 2021

@davimacedo @TysonAndre are you able to look this over?

Copy link

@TysonAndre TysonAndre left a comment

Choose a reason for hiding this comment

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

lgtm other than code style nits

},

set interruptionLevel(value) {
if(typeof value === "string" || value === undefined) {
Copy link

@TysonAndre TysonAndre Nov 14, 2021

Choose a reason for hiding this comment

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

Suggested change
if(typeof value === "string" || value === undefined) {
if (typeof value === "string" || value === undefined) {

nit: use the same spacing used elsewhere

EDIT: oh, it was already inconsistent but if ( seems several times more common. Didn't realize that.

describe("setInterruptionLevel", function () {
it("is chainable", function () {
expect(note.setInterruptionLevel("the-interruption-level")).to.equal(note);
expect(compiledOutput()).to.have.nested.property("aps.interruption\-level", "the-interruption-level");
Copy link

@TysonAndre TysonAndre Nov 14, 2021

Choose a reason for hiding this comment

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

Suggested change
expect(compiledOutput()).to.have.nested.property("aps.interruption\-level", "the-interruption-level");
expect(compiledOutput()).to.have.nested.property("aps.interruption-level", "the-interruption-level");

nit: unnecessary escaping

EDIT: oh, based on existing test case with unnecessary escaping.

@davimacedo
Copy link
Member

@TysonAndre since the code nits are existing one, I'm going to merge this one. Maybe we should install lint/prettier to this repo. What do you think?

@davimacedo davimacedo merged commit b27fa32 into parse-community:master Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants