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

CompatHelper: bump compat for ProtoBuf to 0.11, (keep existing compat) #42

Merged

Conversation

github-actions[bot]
Copy link
Contributor

This pull request changes the compat entry for the ProtoBuf package from 0.7, 0.8 to 0.7, 0.8, 0.11.
This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry.
It is your responsibility to make sure that your package tests pass before you merge this pull request.

@vchuravy vchuravy merged commit 5872757 into master Dec 24, 2021
@NHDaly NHDaly deleted the compathelper/new_version/2021-12-23-01-08-06-180-04204368971 branch December 27, 2021 00:42
@NHDaly
Copy link
Member

NHDaly commented Dec 27, 2021

did the tests run on this PR, @vchuravy?

I saw that the 0.9 and 0.10 upgrade PRs had broken builds due to a change in the proto package:
#34 (comment)
#35 (comment)

I can't tell but it looks like maybe tests didn't run here?

@NHDaly
Copy link
Member

NHDaly commented Dec 27, 2021

maybe we need to double check our CI config... 🤔

@NHDaly
Copy link
Member

NHDaly commented Dec 27, 2021

Mmm yeah, i think this broke master:
https://github.com/JuliaPerf/PProf.jl/runs/4628232421?check_suite_focus=true

I'll revert it.

@NHDaly
Copy link
Member

NHDaly commented Dec 27, 2021

maybe we need to double check our CI config... 🤔

Huh, i don't see anything obvious in the CI config why it didn't run tests.... and the rollback PR has tests running:
#43
https://github.com/JuliaPerf/PProf.jl/runs/4638518767?check_suite_focus=true

@NHDaly
Copy link
Member

NHDaly commented Dec 27, 2021

@vchuravy: @dewilson (from RAI) would like us to add support for proto 0.11, because she wants us to benchmark an internal implementation of data persistence with the ProtoBuf.jl package.

But apparently she's discovered that the later versions (@dewilson: is it starting in 0.9?) of ProtoBuf.jl switch from generating static struct files like we saw to instead generating fully generic proto definitions that use Dicts internally.

We are concerned about the perf characteristics of allocating one Dict per proto message.. So we're a bit sketched about upgrading, and still trying to understand more at the moment.

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