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.

Sign up for GitHub

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

Jump to bottom

Add ESLint rules to mandate braces around if-else and switch-case #3386

Closed
dhruvkb opened this issue Nov 22, 2023 · 9 comments · Fixed by #3474
Closed

Add ESLint rules to mandate braces around if-else and switch-case #3386

dhruvkb opened this issue Nov 22, 2023 · 9 comments · Fixed by #3474
Assignees
@bhavik001
Labels
💻 aspect: code Concerns the software code in the repository ✨ goal: improvement Improvement to an existing user-facing feature 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: mgmt Related to repo management and automations

Comments

@dhruvkb
Copy link
Member

dhruvkb commented Nov 22, 2023

Problem

Currently it's possible to write one-line if-else statements without braces. It's also possible to write case blocks inside switch without braces.

Description

The lack of braces is easier to read initially but makes it possible to make errors when adding more statements to these constructs or updating code in the future.

Mandating braces with ESLint (with the possibility to autofix can prevent these problems.

Alternatives

Additional context

This scope for improvement to our ESLint config was raised in the review for #3375.
@dhruvkb dhruvkb added 🟩 priority: low Low priority and doesn't need to be rushed ✨ goal: improvement Improvement to an existing user-facing feature 💻 aspect: code Concerns the software code in the repository 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: mgmt Related to repo management and automations labels Nov 22, 2023
@dhruvkb dhruvkb changed the title Add ES lint rules to mandate braces around if-else and switch-case Add ESLint rules to mandate braces around if-else and switch-case Nov 22, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Nov 22, 2023
@bhavik001
Copy link
Contributor

Hello @dhruvkb !
Can you assign me this issue I am interested in working on it!

@bhavik001 bhavik001 mentioned this issue Nov 22, 2023
Closed
6 tasks
@bhavik001
Copy link
Contributor

Hey @dhruvkb !
Can you please review the new update that I have added? And let me know if I have to make any changes then.

@dhruvkb
Copy link
Member Author

dhruvkb commented Nov 28, 2023 •
edited
Loading

@bhavik001 your last PR #3387 was closed by you, do you intend to continue working on this issue and open a new PR?

@bhavik001
Copy link
Contributor

Hey @dhruvkb I will gonna create new pull request.

@bhavik001 bhavik001 mentioned this issue Dec 5, 2023
Closed
6 tasks
@bhavik001
Copy link
Contributor

Hey @dhruvkb ! can you check my pull request. And Let me know if I have to update anything.

@openverse-bot openverse-bot moved this from 📋 Backlog to 🏗 In Progress in Openverse Backlog Dec 5, 2023
@bhavik001
Copy link
Contributor

Hey @dhruvkb !
Can you please review the code that I have added in packages\eslint-plugin\src\rules\analytics-configuration.ts

@dhruvkb
Copy link
Member Author

dhruvkb commented Dec 5, 2023

Hey @bhavik001 I've reviewed the PR and suggested some changes. It's really great and proceeding in the right direction but it still needs some additional work. Let's continue this discussion regarding the PR there.

@bhavik001 bhavik001 mentioned this issue Dec 6, 2023
Merged
4 tasks
@bhavik001
Copy link
Contributor

Hey @dhruvkb !
I have added the new pull request with the function working correctly.

@bhavik001
Copy link
Contributor

Hey
Let me know if everything is good or if I have to make any changes. Also can you merge the code after reviewing?

@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Dec 7, 2023
@openverse-bot openverse-bot moved this from ✅ Done to 🏗 In Progress in Openverse Backlog Dec 7, 2023
@sarayourfriend sarayourfriend moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bhavik001 bhavik001

Labels
💻 aspect: code Concerns the software code in the repository ✨ goal: improvement Improvement to an existing user-facing feature 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend 🧱 stack: mgmt Related to repo management and automations
Projects
Openverse Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enforce braces for if-else and switch-case bhavik001/openverse
2 participants
@dhruvkb @bhavik001