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

refactor: enforce linter rules #336

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

refactor: enforce linter rules #336

wants to merge 17 commits into from

Conversation

ethicnology
Copy link

@ethicnology ethicnology commented Nov 28, 2024

In this pull request, I've tried to solve most of the issues, warnings and errors related to linter rules in the repository lib.

Linter rules ensure the code quality among developers and contributors, especially in the context of an open-source project. These coding standards also improve readability.

Regarding the methodology, for each rule exception removed, I've implemented the fixes. This backbreaking work should be review since it can introduce bugs, the project build.

PS: I have excluded test and widget_catalog because they are errored / not updated.

EDIT: I recommend to the reviewer to watch commit by commit, they are atomic

@ethicnology ethicnology requested a review from i5hi November 28, 2024 16:38
@ethicnology
Copy link
Author

We could have a CI that enforce the linter before merging into main

@mocodesmo
Copy link
Collaborator

@ethicnology pls undo the refactor for curly braces and the no wildcard variable uses

for curly braces - if i can have 2 lines for a simple ifelse condition why do i need 6 lines

for no wildcard - using e instead of _ isn't really improving readability and it will also interfere with other wildcard features in dart such as pattern matching and new features like wildcard-variables introduced in dart 3.6

all the other linter rules make sense

@ethicnology
Copy link
Author

@mocodesmo

for no wildcard - using e instead of _ isn't really improving readability and it will also interfere with other wildcard features in dart such as pattern matching and new features like wildcard-variables introduced in dart 3.6

Nice, I got the 3.6 update this morning. They haven’t mentioned the wildcard feature yet, but it’s listed in the 3.7 changelog for the next release. To prepare for this future version, I can revert the refactor: enforce no_wildcard_variable_uses

for curly braces - if i can have 2 lines for a simple if else condition why do i need 6 lines

This isn’t about whether curly braces are better or not but about ensuring all developers follow the same rules for consistency across the codebase. The purpose of a these rules is to provide a shared standard, making the code more maintainable and avoiding unnecessary debates over style. Since we’re building a shared standard to collaborate effectively, what’s @i5hi ’s opinion on curly braces? It might help us resolve this debate more easily.

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.

2 participants