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

feat!: Upgrade to DCM 1.21.0 #658

Merged
merged 3 commits into from
Aug 15, 2024
Merged

feat!: Upgrade to DCM 1.21.0 #658

merged 3 commits into from
Aug 15, 2024

Conversation

witwash
Copy link
Contributor

@witwash witwash commented Aug 12, 2024

Summary

  • Upgraded to 1.21.0 (1.19 -> 1.20 was a pricing change version)
  • fixed linter issues

New enabled rules:

New unused rules:

Testing steps

None

Follow-up issues

None

Check during review

  • Verify against Jira issue.
  • Is the PR over 300 additions? Consider rejecting it with advice to split it. Is it over 500 additions? It should definitely be rejected.
  • Unused code removed.
  • Build passing.
  • Is it a bug fix? Check that it is covered by a proper test (unit or integration).

@witwash witwash marked this pull request as ready for review August 12, 2024 13:12
@witwash witwash requested a review from a team as a code owner August 12, 2024 13:12
@witwash witwash requested a review from ookami-kb August 12, 2024 13:12
@ookami-kb
Copy link
Contributor

prefer-boolean-prefixes

I think it's pretty useful and follows our convention.

newline-before-method

I would enable that.

@witwash
Copy link
Contributor Author

witwash commented Aug 12, 2024

I was doubting about those two. I'll have to do a breaking refactor in optimus because some parameters are not following this exact convention

@witwash
Copy link
Contributor Author

witwash commented Aug 12, 2024

Here is the list of the default prefixes defined in this rule: [is, are, was, were, has, have, had, can, should, will, do, does, did] do we want to add something? @alboiuvlad29 @chaeMil @ookami-kb

Copy link
Contributor

Choose a reason for hiding this comment

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

On a side note would you be able to bump http in remote logger if you get the chance? It's quite outdated and forces us to override the version for kiosk. It also seems to keep throwing errors in development with not being able to find the domain.

@witwash
Copy link
Contributor Author

witwash commented Aug 14, 2024

@ookami-kb,
OK, so, I've done this one with 3 commits because it is a breaking change for optimus, but it is not breaking for remote_logger and kiosk_mode

@witwash
Copy link
Contributor Author

witwash commented Aug 14, 2024

Look like 1.21.1 is already out, but unused _ is still flagged. Oh well 😢

@@ -54,6 +54,7 @@ class BaseDropdownTile extends StatelessWidget {
padding: EdgeInsets.only(right: tokens.spacing200),
child: CheckboxTick(
isEnabled: true,
// ignore: prefer-boolean-prefixes, DCM bug
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just add _ to ignored-names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ignored-names is for methods

Set ignored-names (default is empty) to ignore specific function / method names.

I've tried, no luck

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I've reported it back to DCM, let's have it ignored for now.

@witwash witwash merged commit 800a3a3 into master Aug 15, 2024
6 checks passed
@witwash witwash deleted the DX-2112 branch August 15, 2024 10:43
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