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

Add curly rule for blocks #1836

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Conversation

Krisztiaan
Copy link
Contributor

@Krisztiaan Krisztiaan commented Feb 22, 2021

ESlint rule against one-liners, see thread below.

As for the braces for blocks, this should probably be enforced via eslint, because my (and potentially others') config defaults to one-liners.

💯 I was actually surprised it wasn't already a rule

Originally posted by @Tobbe in #1824 (comment)

@github-actions
Copy link

github-actions bot commented Feb 22, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/create-redwood-app-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-api-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-api-server-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-auth-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-cli-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-core-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-dev-server-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-eslint-config-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-eslint-plugin-redwood-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-forms-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-internal-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-prerender-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-router-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-structure-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-testing-0.25.1-0c92497.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1836/redwoodjs-web-0.25.1-0c92497.tgz

Install this PR by running yarn rw upgrade --pr 1836:0.25.1-0c92497

@Tobbe
Copy link
Member

Tobbe commented Feb 22, 2021

Oops. That was a bigger change than I anticipated. Mostly in the structure package.

@peterp What do you think?

@thedavidprice
Copy link
Contributor

Thanks, @Krisztiaan!

Looked through about half and agree this improves readability. I defer to PP for final say.

@Tobbe
Copy link
Member

Tobbe commented Feb 23, 2021

Looked through about half and agree this improves readability.

It also makes it a lot easier and safer to add more code to a branch of the if/else-statement.

@peterp
Copy link
Contributor

peterp commented Feb 23, 2021

Love it 👍

@thedavidprice
Copy link
Contributor

Hi @Krisztiaan I'd like to get this into the next release. There's a merge conflict from Peter's PR here #1827

Didn't want to assume I could just move your change to .eslintrc over to the new .eslintrc.js. Guessing there might be some other updates as well. Sorry for the double work. Appreciate the help!

@Krisztiaan
Copy link
Contributor Author

@thedavidprice Yeah you could. Rebased, should be g2g now.

@thedavidprice thedavidprice merged commit c64bceb into redwoodjs:main Feb 24, 2021
@thedavidprice thedavidprice added this to the next release milestone Feb 24, 2021
@Krisztiaan Krisztiaan deleted the lint/curly branch February 24, 2021 18:58
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