Skip to content

feat(deriv_lint): update lint rules#587

Open
ahrar-deriv wants to merge 2 commits into
deriv-com:masterfrom
ahrar-deriv:deriv-lint/feat/update
Open

feat(deriv_lint): update lint rules#587
ahrar-deriv wants to merge 2 commits into
deriv-com:masterfrom
ahrar-deriv:deriv-lint/feat/update

Conversation

@ahrar-deriv

@ahrar-deriv ahrar-deriv commented May 16, 2024

Copy link
Copy Markdown
Contributor

Clickup link: https://app.clickup.com/t/20696747/DERG-2271

This PR contains the following changes:

  • updated to the latest researched lint rules
  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Developers Note (Optional)

Pre-launch Checklist (For PR creator)

As a creator of this PR:

  • ✍️ I have included clickup id and package/app_name in the PR title.
  • 👁️ I have gone through the code and removed any temporary changes (commented lines, prints, debug statements etc.).
  • ⚒️ I have fixed any errors/warnings shown by the analyzer/linter.
  • 📝 I have added documentation, comments and logging wherever required.
  • 🧪 I have added necessary tests for these changes.
  • 🔎 I have ensured all existing tests are passing.

Reviewers

@sahani-deriv @ramin-deriv @abedelaziz-deriv

Pre-launch Checklist (For Reviewers)

As a reviewer I ensure that:

  • ✴️ This PR follows the standard PR template.
  • 🪩 The information in this PR properly reflects the code changes.
  • 🧪 All the necessary tests for this PR's are passing.

Pre-launch Checklist (For QA)

  • 👌 It passes the acceptance criteria.

Pre-launch Checklist (For Maintainer)

  • [MAINTAINER_NAME] I make sure this PR fulfills its purpose.

@ahrar-deriv ahrar-deriv changed the title feat: update lint rules feat(deriv_lint): update lint rules May 16, 2024
- noop_primitive_operations
- null_check_on_nullable_type_parameter
- null_closures
- omit_local_variable_types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this conflicts with always specify type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I dont think so , the omit_local_variable_types is just for local variable . but the always specify is for outside the scope vars

- always_put_control_body_on_new_line
- always_put_required_named_parameters_first
- always_require_non_null_named_parameters
- always_specify_types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is a great rule for code readability

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Details
#
NOTE: This rule is removed in Dart 3.3.0; it is no longer functional.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment is about always_specify_types @ahrar-deriv . Is there a reason to remove it?

- use_enums
- use_full_hex_values_for_flutter_colors
- use_function_type_syntax_for_parameters
- use_if_null_to_convert_nulls_to_bools

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

don't you think it is cleaner the other way around?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ahrar-deriv ahrar-deriv May 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the base for our projects. if any rule does not make sense to the team they can
alter it like this

rules:
       use_if_null_to_convert_nulls_to_bools: false

overall I think it is good that we are doing it in null-aware fashion

@abedelaziz-deriv abedelaziz-deriv May 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the link. I know it is effective dart, but do you think it is cleaner to assign a value instead of == true/false?
Their rules can change from bad to good overnight, it is up to us to decide what is best :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes all are subjective. we can vote on it :) all the rules are here for debate

@aliakbar-deriv

Copy link
Copy Markdown
Contributor

@claude

@claude

claude Bot commented Sep 12, 2025

Copy link
Copy Markdown

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

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.

3 participants