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

Eslint #104

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Eslint #104

merged 1 commit into from
Aug 27, 2019

Conversation

retorquere
Copy link
Contributor

builds on #103 -- if so desired, I can rebase onto #102 after that has been merged.

@retorquere
Copy link
Contributor Author

This has now been updated to master. The eslint config makes sure there's a consistent coding style, but will also flag problems like functions being undefind, vars being unused, etc. I've not enabled. There's still two eslint errors that I don't know how to deal with in bootstrap.js, but I don't know what those variables do, so I don't want to touch them.

@retorquere
Copy link
Contributor Author

I'd usually also turn on no-var because the var scoping rules are so strange, but I'll leave that to you to decide.

@wshanks
Copy link
Owner

wshanks commented Aug 25, 2019

What steps did you use to produce these changes? eslint --fix plus some manual fixes for other issues flagged by eslint? Anything else? I am pretty behind on JS tooling -- the last thing I had been using was jscs which it seems was folded into eslint several years ago.

@retorquere
Copy link
Contributor Author

Exactly that - eslint --fix and then manual fixes. There's two remaining eslint complaints that I don't know how to deal with because I don't know where those constants are used/introduced.

@wshanks
Copy link
Owner

wshanks commented Aug 26, 2019

What about the .xul files? Were those formatted with a different tool? It seems the & references to strings in the .dtd file were stripped out. Also, Zotero does not like the blank lines at the top.

@retorquere
Copy link
Contributor Author

Those were done with xmllint -- sorry about that. I've reset them to the upstream master

@wshanks wshanks merged commit 3e2b413 into wshanks:master Aug 27, 2019
@wshanks
Copy link
Owner

wshanks commented Aug 27, 2019

The only changes to your branch that I made were to substitute tabs for spaces in the .xul files and squash the commits.

I feel like there is probably a way to feed the .dtd file into xmllint, but I couldn't figure it out playing with the different dtd arguments.

@retorquere retorquere deleted the eslint branch August 27, 2019 14:21
@bjohas
Copy link
Contributor

bjohas commented Aug 27, 2019

Thank you!

@retorquere
Copy link
Contributor Author

@willsALMANJ you can feed the dtd, but it'd probably replace them with their content as it reformats them.

@wshanks
Copy link
Owner

wshanks commented Aug 27, 2019

FYI: I made a new release with all the recent changes, plus shortcuts for the new functions. I did some other non-functional changes to clean up the code like removing old code related to Firefox features, addons.mozilla.org hosting, and Zotero 4 support.

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.

3 participants