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

Package update with lint rules #70

Closed
wants to merge 0 commits into from
Closed

Package update with lint rules #70

wants to merge 0 commits into from

Conversation

pythonhubdev
Copy link
Contributor

@pythonhubdev pythonhubdev commented Jan 10, 2024

  • Updated packages to latest version
  • Added lint rules
  • Updated the example app with latest flutter version

@lig
Copy link
Contributor

lig commented Jan 10, 2024

Personally, I would like to see those things being a separate PRs.

  • Updating dependencies is a routine operation that could be implemented separately.
  • Fixing documentation grammar could go in it's own separate PR to prevent it be blocked by less trivial changes.

Also, IMO, linting rules shouldn't change current code style drastically but rather embrace it. I mean, if project mostly uses single quotes that what should we put in as a linting rule.

@pythonhubdev
Copy link
Contributor Author

Sure will do as required @lig

@vkammerer
Copy link
Collaborator

@pythonhubdev I think you split your original PR into too many PRs now: They can't pass the tests individually.
It would be ideal if you only took out the "grammar" fixes in a PR, and left all the rest in a single PR.

So could you open one PR with the contents of #71, #72 and #74 ?

I've checked out your branch https://github.com/pythonhubdev/import_sorter/tree/github_actions and I can see that we'll have to add a few changes to make sure the tests pass and to merge. But I'd rather comment on the PR you'll create than to spread comments across multiple ones

This was referenced Jan 12, 2024
@pythonhubdev
Copy link
Contributor Author

Sure will do it

@pythonhubdev
Copy link
Contributor Author

#75 please check this PR

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