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

Make it configurable (:grimacing:) #127

Closed
wants to merge 2 commits into from

Conversation

saveman71
Copy link
Contributor

Hello!

In order to ease installation in a larger project, it might make it easier to be able to only enable some chuncks of the styler at a time, and/or only for some files.

This addresses that.

I know this essentially breaks core values of elixir-styler! No worries if this never get merged, at least it could be a starting point for others.

Tests could be better as well, they're kinda "there" as I didn't find a better place / didn't want to create a file. Don't hestitate to take over / cherrypick what's good if you feel like merging this!

Cheers

@novaugust
Copy link
Contributor

thanks saveman, but as you're expecting i'll decline :) like you said, hopefully this helps others (there's at least half a dozen forks that have reimplemented this)

for folks who come by, the reasons i won't add configuration to styler are:

  1. keeping styler's api / contract with users minimal to ease maintenance
  2. credo style configuration (module on/off) is painting with too broad of a brush to be correct
  3. (least important but a factor!) we built the library as an internal tool. selfishly, we want all the rules on, so there's no personal payoff for adding a way to turn them off, but there is a cost to it (see 1)

that was the tldr, here's the big words :)

api growth

configuration adds a new api/contract i have to maintain as the author of the library. while the implementation is trivial enough, it changes how i and others can contribute to styler in the future.

right now, styler's contract is simply "it's a formatter plugin". by configuring the library to take internal (private) module names as its configuration, those modules become part of the contract. i can no longer rename a module, consolidate multiple modules into one (like I'm likely to do with the Deprecations style soon), or move rewrites into new modules without triggering a breaking change for users.

what's the behaviour when i add new rewrites? should i always add a new module for them, so as to not change how an existing users expect their enabled: ... rules to work?

styler doesnt think in modules

styler's speed results in part from doing lots of things at once; it's not like credo where modules are 1-1 with styles, so switching them on/off would be a bit misleading for users. they're mainly arbitrary ways for me to group code so i don't have a 2000 line module. it's not that styler will never have configuration, it's just that for its configuration to be what i'd see as "correct" would require something a lot more fine-grained. for example, some rewrites in SingleNode rely on the fact that Pipes has been run, or they'll else they'll be doing invalid rewrites.

so, keeping a tight and minimal api keeps the project maintainable (& thus enjoyable) for me

cheers, and thanks again for the contribution ❤️

@novaugust novaugust closed this Jan 26, 2024
@saveman71
Copy link
Contributor Author

thanks for the write up, I can only respect your position and I also believe that's how great OSS exists, by staying strict ;) hopefully the next guy sees the PR instead of creating another fork 😅

@novaugust novaugust mentioned this pull request May 29, 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