Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove unused args, cleanup #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CutOffFrequency
Copy link

@CutOffFrequency CutOffFrequency commented Oct 26, 2019

Removed unused args
Styling changes for consistency (mostly white space: indentation, if() vs if (), trailing spaces, etc)

I tried to keep this light, there are other changes to content.js I considered:

  • replacing the var declarations with block scoped (let and const) declarations (I wasn't sure if this was intentional to support chrome versions <40)

  • removing variable assignment from while conditions in removeHighlight (line 98), I appreciate conciseness but I think you're losing readability here

  • I think how you're iterating through nodes in selectNode (line 131) could probably be improved (at least to lower the line length) but I think the readability is ok here so maybe this is moot

your thoughts?

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.

1 participant