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

Limit rule perfectionist/sort-modules to functions and classes #33

Merged
merged 3 commits into from
Mar 26, 2025

Conversation

mangs
Copy link
Contributor

@mangs mangs commented Mar 26, 2025

Pull Request Checklist

  • I have read the CONTRIBUTING document
  • Readme and changelog updates were made reflecting this PR's changes
  • Increase the project version number in package.json following Semantic Versioning

Changes Included

  • Version bump to 2.4.0
  • Disable rule perfectionist/sort-modules because it was added in Perfectionist version 4's recommended rules; linting friction should be reduced as a result.
  • Upgrade dependencies to latest versions

@mangs mangs requested a review from jduthon as a code owner March 26, 2025 00:11
@jduthon
Copy link
Contributor

jduthon commented Mar 26, 2025

Rethinking this, I have a doubt 😅

Reading the PR description, it sounds like we were already using perfectionist/sort-modules, but I think that's not the case, right?

Thinking about how I would generally go about structuring a file in terms of functions, I'd rarely want to go alphabetical but instead either:

  1. Group things that work together in blocks that work together (e.g. in a utils file)
  2. Order from lowest level to highest level, slowly building the picture of what a specific function broken down to smaller units does

I'd find both of those superior to strict alphabetical ordering (despite they are hard to keep consistent).

Maybe we can run this by the team to gauge everyone's feelings on the topic, I might be in the minority.

@jduthon jduthon self-requested a review March 26, 2025 08:20
@mangs
Copy link
Contributor Author

mangs commented Mar 26, 2025

Reading the PR description, it sounds like we were already using perfectionist/sort-modules, but I think that's not the case, right?

Good question. It comes as a default part of the /recommended rules as of Perfectionist version 4. So just further tweaking it here.

Maybe we can run this by the team to gauge everyone's feelings on the topic, I might be in the minority.

Good idea. I'll bring it up at standup. Thanks for taking a look.

@mangs mangs added question Further information is requested and removed question Further information is requested labels Mar 26, 2025
Copy link
Contributor

@jduthon jduthon left a comment

Choose a reason for hiding this comment

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

Looks great 🚀

@mangs mangs merged commit e6a373d into babbel:main Mar 26, 2025
1 check passed
@mangs mangs deleted the limit-sort-modules branch March 26, 2025 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants