Skip to content

Update JS Guidlines #171

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

Merged
merged 4 commits into from
Apr 9, 2021
Merged

Update JS Guidlines #171

merged 4 commits into from
Apr 9, 2021

Conversation

ascott18
Copy link
Contributor

@ascott18 ascott18 commented Apr 8, 2021

  • Add ESLint rule recommendations
  • Remove all the references to Drupal
  • Remove things that are obsolete in 2021.
  • Fix inconsistent spacing and brace style that violated the very rules that the document was describing.

ascott18 added 2 commits April 8, 2021 10:04
- Add ESLint rule recommendations
- Remove all the references to Drupal
- Remove things that are obsolete in 2021.
@ascott18
Copy link
Contributor Author

ascott18 commented Apr 8, 2021

I'd also like to propose a bigger change here - that we rewrite this document entirely as just a list of ESLint rule recommendations, with links to the corresponding ESLint pages that already contain very good descriptions of why the rule is a good idea as well as examples of conforming code and violating code.

Copy link
Member

@adamskt adamskt left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the newline issues with the code blocks, a lot of the formatting got borked when copying from Google Docs (via Word) to Markdown. I like the idea of referencing ESLint rules for things that line up with the recommendations-- too bad there isn't a linter emojii 😁

This LGTM :shipit:

@SteveByerly
Copy link
Contributor

It would be even nicer to publish a set of ESLint shareable configurations for different applications (js, ts, vue, react, etc). This docs would then mostly list the rules and link back to their source.

@Steborino
Copy link

Agreed about ESLint in general. I suggested a solution back in this Yammer post, but it looks like Shareable Configs is a better implementation of the same idea.

Copy link
Contributor

@SteveByerly SteveByerly left a comment

Choose a reason for hiding this comment

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

Couple of questions, but otherwise looks good to me

Co-authored-by: Steve Byerly <1393464+SteveByerly@users.noreply.github.com>
@SteveByerly
Copy link
Contributor

Agreed about ESLint in general. I suggested a solution back in this Yammer post, but it looks like Shareable Configs is a better implementation of the same idea.

Publishing shareable configs based on IntelliTect coding standards was the goal of my tidier repo, but we never did anything with it.

@adamskt adamskt merged commit 25ddd6e into main Apr 9, 2021
@adamskt adamskt deleted the ascott18/js-review branch April 9, 2021 14:23
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