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: Simplify import_sorter command #76

Merged
merged 4 commits into from
Jan 23, 2024
Merged

feat: Simplify import_sorter command #76

merged 4 commits into from
Jan 23, 2024

Conversation

mrgnhnt96
Copy link
Contributor

The current command to run import sorter is

# as a dev dependency
dart run import_sorter:main

# globally
dart pub global run import_sorter:main

With this change, the :main can be omitted, making the command more intuitive

# as dev dependency
dart run import_sorter

# globally
dart pub global run import_sorter

The :main command will not be removed to avoid breaking existing projects using import_sorter, but could be considered in the future. I've added a deprecated warning whenever import_sorter:main is used.

image

@vkammerer
Copy link
Collaborator

Thanks @mrgnhnt96 for the PR, I think it's a good idea.

But if the idea is to deprecate the :main syntax in a future version, could you please redo your PR this way:

  • rename bin/main.dart to bin/import_sorter.dart in a first commit.
  • then do a second commit that adds a new bin/main.dart that points to bin/import_sorter.dart, and also includes the deprecation warning.

Then please force push your branch.

That way, when we remove support for the :main syntax, we'll just have to delete the bin/main.dart file. It will also keep the deprecation warning isolated, not mixed with the rest of the logic.

@vkammerer
Copy link
Collaborator

Also, please add a third commit that removes references to :main from the documentation. See all current occurrences at https://github.com/search?q=repo%3Afluttercommunity%2Fimport_sorter%20%3Amain&type=code

@pythonhubdev
Copy link
Contributor

@vkammerer once this is merged can we make a new release to pub.dev?

@vkammerer
Copy link
Collaborator

@mrgnhnt96 Thanks for updating your PR.

Could you look into why the import_sorter Github action is failing?
https://github.com/fluttercommunity/import_sorter/actions/runs/7592158197/job/20790428773

@mrgnhnt96
Copy link
Contributor Author

@mrgnhnt96 Thanks for updating your PR.

Could you look into why the import_sorter Github action is failing? https://github.com/fluttercommunity/import_sorter/actions/runs/7592158197/job/20790428773

I hadn't run import sorter lol, I just fixed it. It should pass now

@vkammerer vkammerer merged commit 966775d into fluttercommunity:master Jan 23, 2024
3 checks passed
@vkammerer
Copy link
Collaborator

Thanks @mrgnhnt96 , that made the tests pass.

I'll update the CHANGELOG and prepare a release to pub.dev

@vkammerer
Copy link
Collaborator

@slightfoot We would like to make a new release to pub.dev. Would you be able to help us with it?

@vkammerer
Copy link
Collaborator

This was finally published on pub.dev, under the version 5.0.0-releasecandidate.1

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