Deprecate Data.Version.versionTags
ClosedPublic

Authored by thomie on Oct 29 2014, 10:15 AM.

Details

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
thomie updated this revision to Diff 1095.Oct 29 2014, 10:15 AM
thomie retitled this revision from to Deprecate Data.Version.versionTags.
thomie updated this object.
thomie edited the test plan for this revision. (Show Details)
thomie updated the Trac tickets for this revision.
thomie updated this revision to Diff 1096.Oct 29 2014, 11:25 AM
thomie edited edge metadata.

Suppress deprecation warning when validating hsc2hs

hvr added subscribers: ekmett, hvr.Oct 29 2014, 11:38 AM

LGTM

But I'd like to see a more verbose commit message though giving a one-sentence motivation for this deprecation (like e.g. the unsound Ord/Eq instances)

Finally, I think @ekmett should sign off on this patch as well...

hvr added a reviewer: ekmett.Oct 29 2014, 11:38 AM

PS: When versionTags gets removed, Version could be turned into a newtype, I think...

This requires a change to Cabal to get merged first.

thomie updated this revision to Diff 1100.Oct 29 2014, 4:27 PM

Longer commit message as by hvr's suggestion. Update cabal submodule.

austin accepted this revision.Oct 29 2014, 10:52 PM
austin edited edge metadata.

LGTM, and the Cabal change went upstream, so we should be good to go.

This revision is now accepted and ready to land.Oct 29 2014, 10:52 PM

@austin: Please wait with landing. It doesn't validate currently.

austin requested changes to this revision.Nov 7 2014, 7:29 AM
austin edited edge metadata.

(Requesting changes to make sure this is notably unready.)

This revision now requires changes to proceed.Nov 7 2014, 7:29 AM
ekmett edited edge metadata.EditedNov 7 2014, 9:47 AM

So if the plan is that in 7.12 we're going to remove versionTags, how do we let users construct a Version now such that they won't have to refactor their code in 7.12?

Usually with a deprecation there is a replacement action you can take, right now we're just telling you for a year that you will have to change your code, but giving no way to change it until 7.12.

libraries/base/System/Info.hs
32

If we're going to remove versionTags, would it not make sense to add a way to construct a version from just a list of version numbers? Otherwise lines like this one will break when 7.12 comes out.

utils/ghc-pkg/Main.hs
496

What becomes the new story for this case in 7.12?

hvr added a reviewer: duncan.Nov 7 2014, 10:22 AM
hvr added a subscriber: duncan.

....adding @duncan as he's been wanting this for ages and maybe he has some suggestions to address @ekmett's concerns

In D395#11149, @ekmett wrote:

So if the plan is that in 7.12 we're going to remove versionTags, how do we let users construct a Version now such that they won't have to refactor their code in 7.12?

Usually with a deprecation there is a replacement action you can take, right now we're just telling you for a year that you will have to change your code, but giving no way to change it until 7.12.

I see your point. The code will have to be refactored twice as it stands.

Just to be clear, the deprecation warning is only shown when you use the function versionTags explicitly. I already made the necessary changes to Cabal for 7.10 not to show them. I don't know how much code there is out there that refers to versionTags explicitly.

thomie updated this revision to Diff 1447.Nov 15 2014, 5:19 AM
thomie edited edge metadata.

Rebase master

@thomie:

I'm mostly just looking for some combinator folks'd be able to switch to for construction, so they can make one change in 7.10 in response to the deprecation and their knowledge of the impending change.

e.g. something like makeVersion :: [Int] -> Version would do. That would make this be an issue users could address once immediately after 7.10 comes out rather than have to wait a year for an aftershock of the same change.

duncan edited edge metadata.Nov 18 2014, 9:01 AM

I agree, we want some constructor function. We might also consider at the same time adding a destructor with a better name than versionBranch, one that matches a new constructor function better.

thomie planned changes to this revision.Nov 19 2014, 10:07 AM

I want Python's default keyword arguments!

class Version():
    def __init__(self, versionBranch, versionTags=[]):
        if versionTags:
            print('warning: versionTags is being deprecated')
        self.versionBranch = versionBranch
        self.versionTags = versionTags
ekmett accepted this revision.Nov 21 2014, 4:03 PM
ekmett edited edge metadata.

I'm going to flag this as accepted.

I'd still like a function to construct it, but I won't block this.

This revision was automatically updated to reflect the committed changes.