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

'msgpack-python' egg was renamed 'msgpack' #115

Merged
merged 3 commits into from
Dec 17, 2018
Merged

'msgpack-python' egg was renamed 'msgpack' #115

merged 3 commits into from
Dec 17, 2018

Conversation

jmuchemb
Copy link
Member

@jmuchemb jmuchemb commented May 31, 2018

@jmuchemb jmuchemb changed the title t'msgpack-python' egg was renamed 'msgpack' 'msgpack-python' egg was renamed 'msgpack' May 31, 2018
@mgedmin
Copy link
Member

mgedmin commented May 31, 2018

Should I retry the failed Python 3.6 tests to see if they're non-deterministic failures? Or is anyone planning to analyze the failure and would rather I didn't destroy the current logs?

(I don't think Travis can let you see the logs of jobs that you've retried. If I'm mistaken, I'd love to hear a correction!)

@icemac
Copy link
Member

icemac commented Dec 3, 2018

@jmuchemb Could you please rebase your PR onto master, as it has now successfully running tests again.

@jmuchemb
Copy link
Member Author

jmuchemb commented Dec 3, 2018

Oh so many failures. We could first require v0.5.6, since the regressions seem to be caused by v0.6.

I have no time to work on ZEO code.

@icemac
Copy link
Member

icemac commented Dec 7, 2018

Using a msgpack version < 0.6 fixes the tests.
Can we merge this PR having a follow-up issue to update to the current version or should the issues be fixed in this PR? Any preference?

@jamadden
Copy link
Member

jamadden commented Dec 7, 2018

If the failures are simple and the fixes obvious, I think it'd be fine to fix them here while also updating the pin.

However, from glancing at the log, neither the failures nor the fixes look simple (to me). In that case, it seems reasonable to merge this simple PR, so that master is working/releasable, and defer the version update to a later PR (so long as there's an issuer so we don't forget).

@icemac icemac mentioned this pull request Dec 14, 2018
@icemac
Copy link
Member

icemac commented Dec 14, 2018

I created #122 for the update to a current msgpack version.
Should this change have a change log entry or is it now ready to merge?

@icemac icemac requested a review from jamadden December 14, 2018 07:06
Copy link
Member

@jamadden jamadden left a comment

Choose a reason for hiding this comment

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

It's only a test dependency, so we could probably get away without a change note. On the other hand, it would be good to record somewhere public that (until #122 lands) using the msgpack support requires such-and-such versions of such-and-such dependencies. Changelog lines are cheap, so I would vote for one. But it doesn't seem like a big deal to me.

@icemac icemac merged commit a2377d1 into master Dec 17, 2018
@icemac icemac deleted the msgpack-rename branch December 17, 2018 07:07
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.

4 participants