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

How should we handle invalid config? #32

Closed
gingermusketeer opened this issue Oct 12, 2017 · 8 comments
Closed

How should we handle invalid config? #32

gingermusketeer opened this issue Oct 12, 2017 · 8 comments

Comments

@gingermusketeer
Copy link
Member

As raised by @bessey, see here, we should probably do something about invalid config that is defined in a .rufo file.

This will help users know that their config is actually doing something.

Some of the options include:

  1. Exiting with an error and message
  2. Formatting and printing a warning
  3. Doing nothing
  4. Adding a check config command --check-config

Thoughts?

@splattael
Copy link
Member

At this stage of rufo (pre 1.0) I'd go with option 1 (early exit with error).

Later (after 1.0), I imagine we could go with option 2.

IMHO, option 3 is out of question. It's too dangerous, I think.

Add --check-config is a good idea in any case ;)

Just my 2 cents.

@mjago
Copy link
Member

mjago commented Oct 13, 2017

The way I see it users shouldn't be penalised by having an out-of-date config by giving them an error and no formatted output (which will happen since options are disappearing). Especially since the old config may well conform to the actual format anyway! I would rather see formatted output with a deprecation warning to stderr - and a quiet command line option to quieten the warning if IDE implementers prefer.

We are after all at least theoretically aiming for a zero config formatter where we would ignore the config altogether.

As the config options reduce in size it may make sense allowing configuration as command line options.

A check config option would be very useful.

@splattael
Copy link
Member

@mjago

The way I see it users shouldn't be penalised by having an out-of-date config by giving them an error and no formatted output (which will happen since options are disappearing). Especially since the old config may well conform to the actual format anyway! I would rather see formatted output with a deprecation warning to stderr - and a quiet command line option to quieten the warning if IDE implementers prefer.

I got your point.

My Angst about warning users ("Your config is deprecated or contains deprecated values") is that no-one actually reads deprecation warnings. (myself included 😨)

As a user, I would be very surprised to see my code changed after upgrading rufo and not adjusting my current rufo config. I'd rather see an error saying "Your config is wrong but I do not touch your code now.".

WDYT?

@mjago
Copy link
Member

mjago commented Oct 13, 2017

How about a config option fail_on_invalid_config :fail/:warn/:ignore which defaults to :warn.

Once we stablise (1.0) perhaps it can default to :fail ?

@lime
Copy link

lime commented Oct 13, 2017

As a user, I would be very surprised to see my code changed after upgrading rufo and not adjusting my current rufo config.

Seeing as upgrading rufo might introduce any number of new formatting features and defaults, I wouldn't consider it too surprising that the code changes.

In fact, the closer we get to a zero config formatter, the more this is to be expected. You'll be trusting the tool to format your code the way it sees fit, as of the current version.

This is how I currently see prettier being used. Update to a new version, run against the codebase, commit all changes. Only if I really disagree with any changes made do I look up whether that behaviour is configurable (or possibly already filed as a regression / bug).

As for the confusion caused by a previously working configuration variable no longer having an effect, I think a warning to stderr provides a good balance. You'll notice the warning if you run rufo even once manually from the command line, but formatting won't mysteriously stop working in your editor / IDE.

@lime
Copy link

lime commented Oct 13, 2017

How about a config option fail_on_invalid_config :fail/:warn/:ignore which defaults to :warn.

Once we stablise (1.0) perhaps it can default to :fail ?

Speaking of 1.0, maybe the config file should be versioned..? In that case it would make sense to require a config update when bumping the major version, but keep working across minor changes.

That may not be a very good fit for rufo if the aim indeed is to get as close to zero-config as possible, so take it with a grain of salt. I just realized I've seen lots of projects introduce version: 2 in their config format to distinguish from the original, non-versioned format. :)

@bessey
Copy link
Member

bessey commented Oct 14, 2017 via email

@bessey bessey mentioned this issue Oct 19, 2017
@gingermusketeer
Copy link
Member Author

This has been resolved in #41 :)

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

No branches or pull requests

5 participants