Logic and style tweaks #67

dxdc · 2020-07-05T16:58:34Z

Fix .gitignore
Add linting to code and address all linting warnings/errors (including removing unused variables)
Simplify logic for doorTargetBias, doorCurrentBias

hjdhjd

Sigh. This is what having a MacBook with one of those blasted keyboards does. Thanks.

hjdhjd

I’m not comfortable with this one. Why force the most recent versions upon all of us if we aren’t ready for them? If there’s a good reason, all for it.

hjdhjd

Thanks for cleaning this one up before I could get to it.

hjdhjd

Give me the argument for why single versus double quotes. Other than visual preference, I don’t see the need here. Thoughts?

hjdhjd

I prefer extra parens where possible for improved readability (line 159 in the base). I’m also not a fan of excess carriage returns (lines 163-169 in your patch). Lines 199+...too many spaces, My coding convention is two spaces (no tabs). I believe that has 3.

dxdc · 2020-07-05T21:27:03Z

@hjdhjd you didn't comment on the individual lines so I don't know what you're referring to? will work on the ones I can though.

hjdhjd · 2020-07-05T21:28:19Z

I barely use half the features on github...so bear with my learning curve here as I figure out the social coding etiquette. 😄

hjdhjd · 2020-07-05T21:37:18Z

So the patches I’m good with: fixing the typos, updating from a series of ifs to a switch/case statement. The rest I have questions about. 😄

Can you break this up into multiple PRs, or, if you prefer to just update this PR and you can be patient with me while I sort out how to cherry pick updates, I’d appreciate it.

dxdc · 2020-07-05T21:57:33Z

@hjdhjd made the following changes:

Give me the argument for why single versus double quotes.

It's a convention in languages that distinguish between these, so it's just a style preference. Used double quotes per single as you requested. For consistency, this was very mixed so you will see some single quotes changed to double.

prefer extra parens where possible for improved readability (line 159 in the base).

Fixed

I’m also not a fan of excess carriage returns (lines 163-169 in your patch).

Fixed

Lines 199+...too many spaces, My coding convention is two spaces (no tabs). I believe that has 3.

There were tabs in the latest commits which I fixed after rebasing against master. The spacing here should be correct now. The indenting had some issues in places before.

It may be easier for you to review the individual files and confirm:

Lastly,

I barely use half the features on github...so bear with my learning curve here as I figure out the social coding etiquette.

This is how to comment on individual lines. Click the "+" icon.

dxdc · 2020-07-05T21:59:08Z

I’m not comfortable with this one. Why force the most recent versions upon all of us if we aren’t ready for them? If there’s a good reason, all for it.

I'm assuming (?) this is referring to package.json. This doesn't force new versions, just checks against them. They are dev dependencies.

hjdhjd · 2020-07-05T22:04:09Z

I’m not comfortable with this one. Why force the most recent versions upon all of us if we aren’t ready for them? If there’s a good reason, all for it.

I'm assuming (?) this is referring to package.json. This doesn't force new versions, just checks against them. They are dev dependencies.

Yeah, but this dev, for instance, isn’t using node 14 on all his platforms. 😄

hjdhjd · 2020-07-05T22:06:32Z

@dxdc So here’s my question - if I merge this PR now, how can I see what are all the changes that are being merged in, since there are now multiple patches on top of patches here?

dxdc · 2020-07-05T22:09:39Z

Yeah, but this dev, for instance, isn’t using node 14 on all his platforms.

Ah :) OK. Updated to the latest v10 package

So here’s my question - if I merge this PR now, how can I see what are all the changes that are being merged in, since there are now multiple patches on top of patches here?

For the diff, you can click on : https://github.com/hjdhjd/homebridge-myq2/pull/67/files
or here:

hjdhjd · 2020-07-05T22:11:36Z

Sorry - brain fart. This happens when you get old. 😄

hjdhjd · 2020-07-05T22:13:02Z

Couple of more questions...so why reduce the import block as much as you chose to? I’m going off of the plugin template that the homebridge folks published.

Why swap out var for let...serves no functional purpose in the code?

whreams · 2020-07-05T22:13:37Z

Been watching for a while now… After a lackluster experience speaking to Chamberlain - very much appreciate the work being done here. Great garage door opener - less than great homekit integration. The work being done here bridges that gap. Sincerely - Thanks !

…

On Jul 5, 2020, at 5:09 PM, dxdc ***@***.***> wrote: Yeah, but this dev, for instance, isn’t using node 14 on all his platforms. Ah :) OK. Updated to the latest v10 package So here’s my question - if I merge this PR now, how can I see what are all the changes that are being merged in, since there are now multiple patches on top of patches here? For the diff, you can click on : https://github.com/hjdhjd/homebridge-myq2/pull/67/files <https://github.com/hjdhjd/homebridge-myq2/pull/67/files> or here: <https://user-images.githubusercontent.com/7200365/86543230-4e135e80-bee2-11ea-9d3d-b65dd6163652.png> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#67 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APK2HVT4FEBTLCQQ5U2FZ2TR2D23BANCNFSM4OQ6QBMA>.

dxdc · 2020-07-05T22:14:39Z

Couple of more questions...so why reduce the import block as much as you chose to? I’m going off of the plugin template that the homebridge folks published.

You should only import what is used in that file. If not used, they should be removed.

Why swap out var for let...serves no functional purpose in the code?

https://medium.com/javascript-scene/javascript-es6-var-let-or-const-ba58b8dcde75

hjdhjd · 2020-07-05T22:16:34Z

Fair enough...and thanks, sincerely, for making this better @dxdc.

hjdhjd · 2020-07-05T22:17:05Z

@whreams Thanks...it’s a fun little side project. Really appreciate the support!

whreams · 2020-07-05T22:21:55Z

Thanks to Chamberlain, it continues to be….. ;)

…

On Jul 5, 2020, at 5:17 PM, hjdhjd ***@***.***> wrote: @whreams <https://github.com/whreams> Thanks...it’s a fun little side project. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APK2HVVGSMFLTSMIUK3NTXTR2D3WZANCNFSM4OQ6QBMA>.

github-actions · 2023-04-16T09:02:39Z

This issue is locked to prevent necroposting on closed issues. Please create a new issue for related discussion, if needed.

dxdc mentioned this pull request Jul 5, 2020

Closing door while it is opening causes unexpected behavior #53

Closed

hjdhjd approved these changes Jul 5, 2020

View reviewed changes

hjdhjd suggested changes Jul 5, 2020

View reviewed changes

hjdhjd approved these changes Jul 5, 2020

View reviewed changes

hjdhjd reviewed Jul 5, 2020

View reviewed changes

hjdhjd suggested changes Jul 5, 2020

View reviewed changes

dxdc added 13 commits July 5, 2020 16:39

Extra spaces in .gitignore

8766957

Add linting to package.json

1a1a2b1

Simplify logic

ceb79cf

Linting

7482948

Update dependencies

2f5266e

Additional linting

d56bb14

Update eslint rules

1d75e0d

Fix typo in comments

f5d723f

Simplify logic

196c009

Use double-quotes, additional linting

ea3724b

Fix typo (its)

81d4eaa

Code review comments

41f1950

Per previous commits

bc9e71e

Fix @types/node version

fdf49f6

hjdhjd merged commit c51f96f into hjdhjd:master Jul 5, 2020

github-actions bot locked and limited conversation to collaborators Apr 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logic and style tweaks #67

Logic and style tweaks #67

dxdc commented Jul 5, 2020

hjdhjd left a comment

hjdhjd left a comment

hjdhjd left a comment

hjdhjd left a comment

hjdhjd left a comment

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

dxdc commented Jul 5, 2020

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

whreams commented Jul 5, 2020 via email

dxdc commented Jul 5, 2020 •

edited

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020 •

edited

whreams commented Jul 5, 2020 via email

github-actions bot commented Apr 16, 2023

Logic and style tweaks #67

Logic and style tweaks #67

Conversation

dxdc commented Jul 5, 2020

hjdhjd left a comment

Choose a reason for hiding this comment

hjdhjd left a comment

Choose a reason for hiding this comment

hjdhjd left a comment

Choose a reason for hiding this comment

hjdhjd left a comment

Choose a reason for hiding this comment

hjdhjd left a comment

Choose a reason for hiding this comment

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

dxdc commented Jul 5, 2020

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

dxdc commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020

whreams commented Jul 5, 2020 via email

dxdc commented Jul 5, 2020 • edited

hjdhjd commented Jul 5, 2020

hjdhjd commented Jul 5, 2020 • edited

whreams commented Jul 5, 2020 via email

github-actions bot commented Apr 16, 2023

dxdc commented Jul 5, 2020 •

edited

hjdhjd commented Jul 5, 2020 •

edited