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

update Compat for rename of Pkg to OldPkg #551

Merged
merged 2 commits into from
May 23, 2018
Merged

Conversation

KristofferC
Copy link
Sponsor Member

also temporarily disable the Project file while things are in flux

Somehow I managed to make the #547 branch close itself... -.-.

@StefanKarpinski, @martinholters

also temporarily disable the Project file while things are in flux
@martinholters
Copy link
Member

LGTM by itself (shame we have to disable the Project.toml), but do we still plan to add something as Compat.Pkg in the (near) future? That would be Pkg3 on 0.7 and ... Base.Pkg on 0.6?

So the idea would be to use

  • Compat.OldPkg if one really needs the old Pkg2 functionality/API, even if that may be of limited value on 0.7
  • Compat.Pkg if one needs a way of accessing very basic packaging functionality (add, dir), as is often needed e.g. for CI, compatible with 0.6, 0.7, and onward.

Doesn't sound too bad to me. But I assume the second use to be more common, so maybe we should skip the deprecation?

And of course, if we agree on this as the way forward, README and tests need adjustment.

@KristofferC
Copy link
Sponsor Member Author

Yes, that makes sense to me. So if this PR is ok for you, I intend to merge JuliaLang/julia#27095 when I fixed CI, then merge this and tag a new Compat, then update the registry and we should be able to release an alpha for 0.7.

@martinholters
Copy link
Member

This PR is ok with me, modulo test and README update.

But should we keep Compat.Pkg undeprecated or not? (Yes, suddenly switching Compat.Pkg from Pkg2 to Pkg3 might be breaking. But a deprecation that tells people to use Compat.OldPkg instead of Compat.Pkg when in the majority of cases, they should soon be using Compat.Pkg again soon does not sound too useful.) Or should we make Compat.Pkg be Pkg3 (when available) immediately?

@KristofferC
Copy link
Sponsor Member Author

But should we keep Compat.Pkg undeprecated or not?

Ok, let's not then. The PR is then as simple as this.

@martinholters
Copy link
Member

martinholters commented May 23, 2018

Sometimes, simple really is best. 😄 And we're surely not doing anything wrong here.

Also, this agrees with what (I perceived) we came to in #547:

  • Being able to access Pkg2 on 0.7 will be of little use, probably no need to support that.
  • We just let Compat.Pkg become Pkg3 and wait for the complaint to come in.

Bottom line: LGTM. Also, no need to wait for JuliaLang/julia#27095 to merge this, or?

@KristofferC
Copy link
Sponsor Member Author

No, this should be good to go then. Thanks for your feedback, greatly appreciated!

@martinholters
Copy link
Member

We probably should wait a little more to give others a chance to chime in? Having a PR open at least 24 hours if not terribly urgent is a nice rule. OTOH, #547 had been open for a while, so 🤷‍♂️

@KristofferC KristofferC merged commit 2a82221 into master May 23, 2018
@martinholters martinholters deleted the kc/Compat_fix branch May 23, 2018 20:30
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