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

Tell users about cabal.project.local~ #6877

Merged
merged 2 commits into from
Jun 6, 2020

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Jun 5, 2020


Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog (add file to changelog.d directory).
  • The documentation has been updated, if necessary.

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

@lukel97 lukel97 force-pushed the cabalprojectlocal-copy-message branch from 36991d0 to f36b53b Compare June 6, 2020 00:05
@@ -1,5 +1,5 @@
# cabal v2-configure
'cabal.project.local' file already exists. Now overwriting it.
'cabal.project.local' already exists, copying it to 'cabal.project.local~'.
Copy link
Collaborator

@phadej phadej Jun 6, 2020

Choose a reason for hiding this comment

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

s/copying/backuping/.

Though I don't remember what happens when cabal.project.local~ already exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just gets overwritten it looks like

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, let us fix that. Otherwise we could just overwrite cabal.project.local directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix by adding a message or by backing up cabal.project.local~ somewhere further?

@lukel97 lukel97 force-pushed the cabalprojectlocal-copy-message branch from f36b53b to 74ff63d Compare June 6, 2020 11:15
@@ -95,7 +95,7 @@ configureAction flags@NixStyleFlags {..} _extraArgs globalFlags = do
-- before overwriting
exists <- doesFileExist "cabal.project.local"
when exists $ do
Copy link
Collaborator

Choose a reason for hiding this comment

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

when exists let's search for the first one of cabal.project.local~, cabal.project.local~0 cabal.project.local~1 etc. until one doesn't.

Note: we need to change this to take into account --project-file as we on it. I wonder if there's an issue about that too.

-- | A custom target problem
| CustomTargetProblem a
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was to fix a haddock failure

@lukel97 lukel97 force-pushed the cabalprojectlocal-copy-message branch from 55f965d to 7357ad9 Compare June 6, 2020 13:07
@lukel97 lukel97 requested a review from phadej June 6, 2020 13:07
Copy link
Collaborator

@phadej phadej 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. Shouldn't foo.project however have something in it? The fact empty project file works is a not right, but fixing that is outside of scope of this PR.

@lukel97 lukel97 force-pushed the cabalprojectlocal-copy-message branch from 7357ad9 to 32c6196 Compare June 6, 2020 15:00
@phadej
Copy link
Collaborator

phadej commented Jun 6, 2020

CI fails for real

@lukel97 lukel97 force-pushed the cabalprojectlocal-copy-message branch from 32c6196 to f9bf6b0 Compare June 6, 2020 17:13
@phadej phadej added this to the 3.4.0.0 milestone Jun 6, 2020
@lukel97
Copy link
Contributor Author

lukel97 commented Jun 6, 2020

CI looks green now

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