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

Add migration guide for #9718 #10578

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ffaf1
Copy link
Collaborator

@ffaf1 ffaf1 commented Nov 22, 2024

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • no Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

Thanks to the excellent doc contribution by @ulidtko in #10559.

@ffaf1
Copy link
Collaborator Author

ffaf1 commented Nov 22, 2024

Does this need a backport? Or do we always link master for changelogs?

PR haskell#9718 and related PRs reshuffled and refactored Cabal API.
This patch adds a simple migration guide for Cabal library
users.

Authored-by: Maxim Ivanov
@ffaf1
Copy link
Collaborator Author

ffaf1 commented Nov 22, 2024

Check commit by commit, GitHub rich diff gets confused and messes the diff up.

@ffaf1 ffaf1 marked this pull request as ready for review November 22, 2024 08:51
Copy link

@ulidtko ulidtko left a comment

Choose a reason for hiding this comment

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

LGTM with a minor nitpick

release-notes/Cabal-3.14.0.0.md Outdated Show resolved Hide resolved
@ulysses4ever
Copy link
Collaborator

Does this need a backport? Or do we always link master for changelogs?

Only master, yes.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Terrific! Thanks Francesco and Maxim!

@ffaf1 ffaf1 added squash+merge me Tell Mergify Bot to squash-merge and removed attention: needs-review labels Nov 22, 2024
locations. (Hopefully, avoiding confusion which things should go where and
how.)

In your specific circumstance, you'll need to decide how much of that nuance
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be pointed out that using getSymbolicPath has some nuance.

  • FilePath in cabal-install is always relative to the project directory which is where cabal-install is run.
  • SymbolicPath is relative to the package root of the particular package Cabal is compiling at that time.

For a ./Setup.hs you are probably fine using interpretSymbolicPathCWD, but that is only because cabal-install calls ./Setup in the package directory. If you are writing an application using Cabal library, you might need to take the working directory into account propertly.

Copy link

Choose a reason for hiding this comment

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

Yep, that basically echoes what the haddocks (+types) say.

I didn't want to overload release notes with deep details; so there's no "do this" code sample, but the call to action is to go check the updated module reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You two have more experience than me with these modules. The warning (“[…] but see the linked module's haddocks for caveats and less drastic options”) seems assertive enough.

Any modification proposal is welcome!

@mpickering
Copy link
Collaborator

Thanks for contributing this @ulidtko

@ffaf1 ffaf1 linked an issue Nov 23, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cabal library API breakage in 3.14.0.0
4 participants