hpack icon indicating copy to clipboard operation
hpack copied to clipboard

Discussion: Fail on unrecognized fields (instead of warn)

Open sol opened this issue 7 years ago • 4 comments

Currently hpack warns on unrecognized fields (unless they start with an underscore). This was originally meant so that older versions of hpack can succeed on a package.yaml that uses fields that were added in a later version. Originally that made a lot of sense as early on many of the fields that I added were non-critical or purely informational and could safely be ignored. However, by now most of the new stuff we add has semantic significance and I'm puzzled whether a hard failure would be preferable to a warning.

Some options:

  1. Fail on unrecognized fields unconditionally.
  2. Fail on unrecognized fields, but allow to succeed with --lenient (which we would add just for this purpose)
  3. Continue to warn on unrecognized fields, but add a --strict flag that turns this (or maybe all) warnings into errors

@mgsloan @snoyberg @tfausak as Hpack is more widely used by now, I would appreciate your feedback on this, esp. regarding stack. A quick thumbs up is sufficient if any of the options work for you.

In general, is it ok with you guys, if I ping you for feedback on more invasive changes in the future? I would also want to give at least one more person upload rights to Hackage (as a backup), volunteers?

sol avatar Feb 02 '18 12:02 sol

I'm weakly :+1: on option 3. I think I would personally always use the --strict flag, but it doesn't feel right as the default. For example, I pretty much always use stack build --pedantic, but I'm glad that isn't the default.

I am happy to be pinged about hpack changes and maintenance! I can't commit any set amount of time or attention to the project, but I keep an eye on it anyway.

I would also be happy to be a co-maintainer on Hackage, but I'm not sure that's necessary. I think it would be better to set up Travis CI to automate Hackage uploads (and binaries #256). That's something I'm pretty familiar with, so I can try to set it up soon.

tfausak avatar Feb 02 '18 12:02 tfausak

I'm not sure there's a blanket prescription here.

I assume that this issue is prompted by #250, but that is a bit of a special case where, effectively, the presence or absence of fields is being used to encode sum types in YAML. Where there's forwards-compatibility risk, as in that case, I think erroring on unknown fields by default is probably a good option.

On the other hand, something like extra-doc-files doesn't have that forward-compatibility worry, and could safely be a warning. Sure, the generated .cabal would be incomplete, but it won't be wrong.

quasicomputational avatar Feb 02 '18 16:02 quasicomputational

I think it would be better to set up Travis CI to automate Hackage uploads (and binaries #256). That's something I'm pretty familiar with, so I can try to set it up soon.

Ok, sounds good!

sol avatar Feb 03 '18 06:02 sol

I assume that this issue is prompted by #250

I have been considering this for a while, it is not prompted by #250.

sol avatar Feb 03 '18 06:02 sol