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

Upgrade to hpack 0.20.0 (fixes #3383) #3579

Merged
merged 1 commit into from Nov 19, 2017

Conversation

snoyberg
Copy link
Contributor

Note: Documentation fixes for https://docs.haskellstack.org/en/stable/ should target the "stable" branch, not master.

Please include the following checklist in your PR:

  • Any changes that could be relevant to users have been recorded in the ChangeLog.md
  • The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

@snoyberg snoyberg force-pushed the 3383-detect-hpack-cabal-changes branch from 7483dd3 to 85cbc6f Compare November 16, 2017 17:09
Hpack.ExistingCabalFileWasModifiedManually -> prettyWarnL
[ cabalFile
, flow "was manually modified but it is autogenerated,"
, flow "please delete and modify package.yaml in the future"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think "please delete it" would be clearer, right now it could read as "delete + modify package.yaml". Perhaps it's not so ambiguous, but still best to be clear as possible

Copy link
Contributor

Choose a reason for hiding this comment

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

Or something along the lines of

"WARNING: $cabalFile was modified manually.  Ignoring package.yaml in favor of $cabaFile.  If you want to use package.yaml instead of $cabalFile then please delete $cabalFile."

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, perhaps! Or it could just pre-emptively delete the cabal file! 👿 Hah probably people wouldn't appreciate that.

Perhaps it should just error in this circumstance. Hmm hmm, nothing seems to be quite perfect.

Copy link
Contributor

Choose a reason for hiding this comment

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

One more note: The reason why I added --force to hpack and why --force is preferable to deleting the cabal file is, that hpack does some effort to minimize diffs if a cabal file is already present. So hpack --force and rm *.cabal && hpack may lead to differently formatted cabal files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go with @sol's verbiage, I think the hpack author is likely the best to figure these things out anyway :)

@mgsloan
Copy link
Contributor

mgsloan commented Nov 16, 2017

LGTM just one comment

@snoyberg snoyberg force-pushed the 3383-detect-hpack-cabal-changes branch from 85cbc6f to 74c7ccd Compare November 19, 2017 10:40
@snoyberg snoyberg force-pushed the 3383-detect-hpack-cabal-changes branch from 74c7ccd to 57af148 Compare November 19, 2017 13:26
@snoyberg snoyberg merged commit 902ff2f into master Nov 19, 2017
@snoyberg snoyberg deleted the 3383-detect-hpack-cabal-changes branch November 19, 2017 16: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

Successfully merging this pull request may close these issues.

None yet

3 participants