Skip to content

refactor: fix analyzer code style warnings #734

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

Merged
merged 16 commits into from
May 30, 2022

Conversation

fischerscode
Copy link
Contributor

@fischerscode fischerscode commented Apr 30, 2022

New Pull Request Checklist

Issue Description

This PR fixes the trivial, non breaking analyzer infos.

Related issue: #733

Approach

Mostly I just let the ide fix it.

TODOs before merging

- [ ] Make sure the change to _ParseLiveListElementWidgetState did not break anything.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Apr 30, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@fischerscode
Copy link
Contributor Author

Each Prefer using lowerCamelCase for constant names. info, might require a breaking change.

@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #734 (aa7c8df) into master (a5224aa) will increase coverage by 6.93%.
The diff coverage is 11.68%.

@@            Coverage Diff             @@
##           master     #734      +/-   ##
==========================================
+ Coverage    3.34%   10.28%   +6.93%     
==========================================
  Files           4       48      +44     
  Lines         269     2810    +2541     
==========================================
+ Hits            9      289     +280     
- Misses        260     2521    +2261     
Impacted Files Coverage Δ
packages/dart/lib/src/network/dio_adapter_io.dart 0.00% <ø> (ø)
packages/dart/lib/src/network/parse_client.dart 20.00% <0.00%> (ø)
...ackages/dart/lib/src/network/parse_dio_client.dart 0.00% <0.00%> (ø)
...ckages/dart/lib/src/network/parse_http_client.dart 5.12% <0.00%> (ø)
...ackages/dart/lib/src/network/parse_live_query.dart 0.91% <0.00%> (ø)
packages/dart/lib/src/objects/parse_base.dart 37.61% <0.00%> (ø)
packages/dart/lib/src/objects/parse_config.dart 0.00% <0.00%> (ø)
packages/dart/lib/src/objects/parse_error.dart 0.00% <ø> (ø)
packages/dart/lib/src/objects/parse_file.dart 0.00% <0.00%> (ø)
packages/dart/lib/src/objects/parse_file_web.dart 0.00% <0.00%> (ø)
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5224aa...aa7c8df. Read the comment docs.

@fischerscode
Copy link
Contributor Author

Merging #734 (769e900) into master (1280ce3) will decrease coverage by 0.03%.

I think due to the introduction of defaultParseFileConstructor.

@mtrezza
Copy link
Member

mtrezza commented May 2, 2022

Kindly request a review when this is ready; not sure if you are currently waiting for review

@fischerscode
Copy link
Contributor Author

I'll have to test, if this really does not break the animation.

Didn't test it yet. I'll request a review when I did.

@fischerscode fischerscode marked this pull request as draft May 3, 2022 11:40
@fischerscode
Copy link
Contributor Author

Make sure the change to _ParseLiveListElementWidgetState did not break anything.

It works as expected, but I'll revert the change since it would be breaking. Older versions of Flutter (before v2.2.0-10.1.pre) require vsync

Continue using vsync in _ParseLiveListElementWidgetState to avoid a breaking change.
@fischerscode fischerscode marked this pull request as ready for review May 20, 2022 14:15
@fischerscode
Copy link
Contributor Author

codecov should be fine. Might be due to more compact format or fewer imports.

@fischerscode fischerscode requested a review from phillwiggins May 20, 2022 14:29
@fischerscode
Copy link
Contributor Author

Sorry @phillwiggins pressed the wrong button.

@fischerscode fischerscode requested review from mtrezza and removed request for phillwiggins May 20, 2022 15:10
@mtrezza mtrezza requested review from a team and removed request for mtrezza May 20, 2022 16:11
@fischerscode fischerscode mentioned this pull request May 22, 2022
2 tasks
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Could you please add the changelog entries to the corresponding packages? Entries should only go to the package in which the changes occur. We do separate releases and versioning now. See #668 (comment)

If this PR does not fix a bug, add a feature or improve performance, but is merely a refactor to get rid of recommended code style changes, then this would not lead to a changelog entry. The PR would simply be merged as something like refactor: fix analyzer code style warnings. I renamed the PR title; if that's incorrect, please rename accordingly.

@mtrezza mtrezza changed the title fix: simple analyzer infos refactor: fix analyzer code style warnings May 29, 2022
@fischerscode fischerscode requested a review from mtrezza May 30, 2022 18:40
@fischerscode fischerscode requested a review from mtrezza May 30, 2022 19:18
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! Let's try out this first dart - flutter combo release!

@mtrezza mtrezza merged commit 8319bf9 into parse-community:master May 30, 2022
@mtrezza
Copy link
Member

mtrezza commented May 30, 2022

Oops, we forgot forgot to bump the package versions. I guess we can just wait for the next PR, probably #738, and do a release. Unless you feel like opening another PR just for the version bump.

@fischerscode
Copy link
Contributor Author

Oops, we forgot forgot to bump the package versions.

Oops...

Unless you feel like opening another PR just for the version bump.

If you want, I can do it. It might be best to test it soon rather then later.

@mtrezza
Copy link
Member

mtrezza commented May 30, 2022

Sure, please go ahead

@fischerscode
Copy link
Contributor Author

Sure, please go ahead

Link it to #733?

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