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

Update and reduction #16

Closed
wants to merge 2 commits into from
Closed

Update and reduction #16

wants to merge 2 commits into from

Conversation

MattWellie
Copy link
Collaborator

@MattWellie MattWellie commented Jun 3, 2024

Looking for some feedback here - this repo was out of date relative to ones with decent activity. This is an update/upgrade(?)

  • isort removed, ruff does that
  • black removed, ruff does that (?) - I've used black forever, but other than a general familiarity with it, I don't see a reason to keep it just for functionality ruff contains
  • markdownlint config & usage removed, ruff does that (?)
  • tool versions bumped

Specifically I'm not sure on the interaction between COM812 and split-on-trailing-comma = false - do we want to split all lists with trailing commas even if they'll fit on one line (for more meaningful git diffs in future), or condense lists that will fit on a single line for overall file size/line count?

supersedes #15? (sorry, hadn't seen that)

Copy link
Collaborator

@illusional illusional left a comment

Choose a reason for hiding this comment

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

I think you've copied a lot from production-pipelines, which has a different from what we'd recommend. I'd want to keep markdownlint, and I'm not 100% confident on removing black, I think I'd like to discuss that more before we move forward with that.

If you can clean-up production-pipeline carryover (removed markdown lint, some large_file exclude, lots of changes in pyproject.toml), and keep black - happy to re-review.

@illusional
Copy link
Collaborator

@MattWellie, otherwise I'm more likely conservitely update #15, merge that and then leave this as a suggestion to discuss.

@MattWellie MattWellie closed this Jun 3, 2024
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