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

Add import sorting to format tool (backpatch to v3.7.1) #381

Merged
merged 25 commits into from
Jul 25, 2022

Conversation

regenvanwalbeek-wf
Copy link
Contributor

@regenvanwalbeek-wf regenvanwalbeek-wf commented Jul 14, 2022

Note: This is a backpatch to 3.7.1 for consumers who have not upgraded to analyzer 1.0.0 - The changes for the mainline branch exist here

Having to manually sort imports is tedious. It would be better if we could just run the formatter to receive sorted imports.

I added an organizeImports flag to the FormatTool. When set to true, it will also run the file through an import sorter.

The import sorter tries to comply with https://dart.dev/tools/linter-rules#directives_ordering

- Reviewer note: This is copied from dart_dev_workiva for easy reviewing.
- This should only contain import changes
- This will allow us to execute multiple processes as
part of a `ddev format` command
@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

pubspec.yaml Outdated Show resolved Hide resolved
@regenvanwalbeek-wf
Copy link
Contributor Author

Will I have to add a skynet.yaml for this to successfully release?

@regenvanwalbeek-wf regenvanwalbeek-wf changed the title Add import sorting to format tool Add import sorting to format tool (backpatch to v3.7.1) Jul 14, 2022
@regenvanwalbeek-wf regenvanwalbeek-wf marked this pull request as ready for review July 18, 2022 14:20
lib/src/tools/format_tool.dart Outdated Show resolved Hide resolved
lib/src/executable.dart Outdated Show resolved Hide resolved
lib/src/utils/import_cleaner/import_cleaner.dart Outdated Show resolved Hide resolved
pubspec.yaml Outdated Show resolved Hide resolved
lib/src/tools/format_tool.dart Outdated Show resolved Hide resolved
lib/src/tools/format_tool.dart Outdated Show resolved Hide resolved
lib/src/utils/organize_imports/import_collector.dart Outdated Show resolved Hide resolved
@evanweible-wf
Copy link
Contributor

@regenvanwalbeek-wf could you add a changelog entry for 3.7.2? I can't quite remember how backpatch releases work, so it's possible this may end up effectively being the release PR. I've updated MARV with the correct version.

@regenvanwalbeek-wf
Copy link
Contributor Author

🔍 Should this be a 3.8.0 since I'm adding a field to the public API? Probably not a huge deal

@evanweible-wf
Copy link
Contributor

evanweible-wf commented Jul 25, 2022

Unfortunately since we're back patching, we're a bit limited since 3.8.x already exists. Ideally we would only be backpatching bugs, but I think we're alright to just call this a patch release anyway.

Edit: what I mean is that since we're adding this public API here and on the latest 3.x branch, it should be relatively safe. Consumers who use the API from 3.7.2 will still have the API when they are able to upgrade to latest.

@regenvanwalbeek-wf
Copy link
Contributor Author

That makes sense. Pushed!

@evanweible-wf
Copy link
Contributor

QA +1

  • CI passes (tests added and updated in this PR)
  • Tested this locally with organizeDirectives = false and organizeDirectives = true

@evanweible-wf
Copy link
Contributor

@Workiva/release-management-p

Copy link
Contributor

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rmconsole5-wk rmconsole5-wk merged commit 31e2eda into v371 Jul 25, 2022
@rmconsole5-wk rmconsole5-wk deleted the import_sort_v371backpatch branch July 25, 2022 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants