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

Linting and code styles #110

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Linting and code styles #110

wants to merge 6 commits into from

Conversation

zzolo
Copy link

@zzolo zzolo commented Jan 3, 2019

Wonderful project, thanks so much for sharing with the world.

This pull requests adds some linting and code style consistency support, which, for neurotic people like me, makes it easier to contribute. I think code styles are great, but I don't want to be preachy about it.

This adds two things:

  1. eslint which will enforce code style while helping to identify errors.
    • Configuration is managed in .eslintrc.json. It builds off the recommended linting from the eslint module, and adds just a couple differences that I think align with the current styles.
    • It also adds an es5 plugin, as I think Illustrator uses an older version of a Javascript engine, but I can't find very definitive documentation about this.
    • Basic checking can be done with npm run lint, which will lint the main ai2html.js and the JS files in tests/.
    • You can also fix in place with npm run lintFix. This will update the files, but will also error, as there are some code that just needs to be updated to pass.
  2. editorconfig which is a low-level configuration around code editors. The main thing it does that is helpful is define spacing, such as two spaces (vs. tabs). See below for how this can be very helpful.

Future considerations

  • I did not actually run the fixing and commit it here, since that would be a very noisy commit.
  • There are plugins for editors to add integration for these tools.
    • editorconfig integrations actually work seamlessly, where pressing "tab" will insert the correct characters defined in the .editorconfig config.
    • eslint integrations should at least highlight issues as you edit, though many editors may have some sort of "format on save" option which may seem extreme, but I find very helpful.
      • Note that, using eslint with prettier can be very helpful for other types of files.
  • Another option may to add a pre-commit git hook to format when making a commit.
  • I didn't add any documentation about this, but would be worth mentioning in a contributors section.

@mbloch
Copy link
Contributor

mbloch commented Jan 3, 2019

Hi, thanks for this contribution! I'll try to review it soon. Meanwhile, I'm interested in whether you've been using ai2html at the Star Tribune. It sounds like you're interested in contributing, do you have some thoughts about improving the tool, other than coding style?
Cheers,
mbloch

@zzolo zzolo mentioned this pull request Jan 4, 2019
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.

None yet

2 participants