Skip to content

Fix regression of fix for #11017 #65

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

Closed
wants to merge 1 commit into from

Conversation

duritong
Copy link
Contributor

ensure should have a default of :present. Unfortunately, the
Ensure class does not do that by default for our type. So we
need to do that here explicitly.

This restores the old (sane) behavior. See part of the discussion in
#36

@jeffmccune
Copy link
Contributor

@duritong Hey, sorry for not getting this merged in. I went ahead and fixed it too, just against the 2.3.x branch rather than master. It's a bug fix so we need to put out a bug fix for the existing release. (stdlib follows semver.org version guidelines)

I think my pull request at GH-69 covers this as well because the spec tests were also failing in 2.3.x. Once merged into 2.3.x it'll get merged up into master.

@duritong
Copy link
Contributor Author

@jeffmccune I merged our diverged branches and included the proper fix as I noted in GH-69

Imho this should now properly fix it.

@jeffmccune
Copy link
Contributor

@duritong Could you rebase this change on the 2.3.x branch so it will get merged up into master?

We need the defaultvalues for that.
@duritong
Copy link
Contributor Author

Here you go. I just made a new commit, instead of merging our 2 diverged trees. This will require some plumbing on my side, but it will apply cleanly for you.

@jeffmccune
Copy link
Contributor

@duritong No, I don't think you understand what I mean.

It's not that any branches have diverged, it's that this is actually a bug in the 2.3.x series and as such the patch needs to be based on the 2.3.x branch.

We have a "merge up" process so the patches to 2.3.x will automatically get merged into master.

As long as you're tracking the master branch in your own fork you won't have diverged from upstream.

Ping me on IRC if you'd like an easy way to rebase this patch on 2.3.x instead of master. I'll be in meetings for the next couple of hours but I should be online again at 2PM PT.

@duritong
Copy link
Contributor Author

Right. Ok so I have a commit based on the 2.3.x branch in my 2.3.x branch pushed. However, I don't seem to be able to figure out how I can adjust this pull request. Let's try to figure that out on IRC. Today, I'm probably available from now on till 4PM PT.

@jeffmccune
Copy link
Contributor

Ah yes, I don't see a way to change a pull request to target a new base branch.

Probably easiest to simply open a new pull request.

@duritong
Copy link
Contributor Author

Ok, will do so.

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