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

protobuf v4.22.5 #186

Merged
merged 12 commits into from
May 23, 2023
Merged

protobuf v4.22.5 #186

merged 12 commits into from
May 23, 2023

Conversation

h-vetinari
Copy link
Member

After conda-forge/libprotobuf-feedstock#144 was finally merged...

Closes #180

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

OK, it looks like we're running into pypa/pip#12010

@h-vetinari
Copy link
Member Author

Ugh...

  $PREFIX/include/absl/strings/cord.h:999:18: error: cannot convert 'absl::lts_20230125::Cord::HashFragmented<absl::lts_20230125::hash_internal::MixingHashState>(absl::lts_20230125::hash_internal::MixingHashState) const::<lambda(int)>' to 'int'
    999 |     ForEachChunk([&combiner, &hash_state](absl::string_view chunk) {
        |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        |                  |
        |                  absl::lts_20230125::Cord::HashFragmented<absl::lts_20230125::hash_internal::MixingHashState>(absl::lts_20230125::hash_internal::MixingHashState) const::<lambda(int)>
   1000 |       hash_state = combiner.add_buffer(std::move(hash_state), chunk.data(),
        |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1001 |                                        chunk.size());
        |                                        ~~~~~~~~~~~~~~
   1002 |     });
        |     ~

xylar
xylar previously requested changes May 13, 2023
Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @h-vetinari! I think we want to keep separate major and lib_major variables in case they diverge again in the future.

recipe/meta.yaml Show resolved Hide resolved
Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I don't think there's a way for me to rescind my request for changes without approving, which doesn't make sense to do while tests are failing. But you may ignore my request.

@h-vetinari h-vetinari changed the title protobuf v4.22.3 protobuf v4.22.5 May 18, 2023
@h-vetinari h-vetinari dismissed xylar’s stale review May 18, 2023 06:28

I don't think there's a way for me to rescind my request for changes

@xylar
Copy link
Contributor

xylar commented May 18, 2023

@h-vetinari, please remove me as a maintainer as part of this PR if you would. It seems like you've got this and my efforts are better spent elsewhere.

@h-vetinari
Copy link
Member Author

@h-vetinari, please remove me as a maintainer as part of this PR if you would. It seems like you've got this and my efforts are better spent elsewhere.

Did you notice that I followed your request for keeping the distinction between the two versions? I cannot stop you from removing yourself if you so choose, but saying that I disagree with you in something shouldn't have to be a reason to throw in the towel. Perhaps you'll reconsider?

@xylar
Copy link
Contributor

xylar commented May 18, 2023

but saying that I disagree with you in something shouldn't have to be a reason to throw in the towel. Perhaps you'll reconsider?

It seemed from your initial response as if you would prefer that other maintainers not request changes to your PRs. I appreciate you listening to my request, but if you prefer to work alone, and that's certainly the impression that I got, I'm happy to oblige.

@h-vetinari
Copy link
Member Author

It seemed from your initial response as if you would prefer that other maintainers not request changes to your PRs. I appreciate you listening to my request, but if you prefer to work alone, and that's certainly the impression that I got, I'm happy to oblige.

I'd very much appreciate others to contribute, none of this is dependent on me specifically - the analysis that libprotobuf depends on abseil test targets, building those, etc. could have well been done by anyone. As it turns out, I was the first and so far only one to dive into this rabbit hole. upb support will be another such effort it seems.

For context about my previous comment: I find a change request unnecessarily... confrontational. Even more so for a relatively inconsequential detail, even more so for a PR that's work-in-progress, and even more so for something in the context of a substantial overarching effort done by someone else. In 99% of the cases, it could just be a comment, then it's much more collaborative. Though I'll concede that, as long as I'm the only one staring into this abyss tackling these problems, my sensitivity is probably somewhat elevated, and it's easier to get discouraged by otherwise trifling aspects.

@xylar
Copy link
Contributor

xylar commented May 18, 2023

@h-vetinari, I see. Thank you for taking the time to explain.

In the team I work with, change request are a standard practice but I could see how they come across as aggressive. And I can see why you got annoyed, especially by my persistence.

I cared about this because I caused a not insignificant mess by misunderstanding the version in the past. My past mistakes are also part of why I'm not sure I can do more good than harm here.

I commented on a work in progress because it isn't standard practice to wait for a review on conda-forge -- it's typically necessary to react quickly or lose your chance.

As to the burden you're shouldering alone, I sincerely wish I could be of more help. I don't have the necessary expertise. I do hope you find folks willing to help who do.

@h-vetinari
Copy link
Member Author

h-vetinari commented May 18, 2023

Alright, this builds except for pypy, which has been disabled upstream though (effectively). I think I'll skip for now. nevermind, found the right patch to backport

@h-vetinari h-vetinari force-pushed the bump branch 2 times, most recently from ff7c09e to b9bdc72 Compare May 18, 2023 08:50
@xhochy xhochy merged commit 9792f63 into conda-forge:main May 23, 2023
@h-vetinari h-vetinari deleted the bump branch May 23, 2023 07:18
@h-vetinari
Copy link
Member Author

@h-vetinari, I see. Thank you for taking the time to explain.

Likewise! :)

I don't have the necessary expertise. I do hope you find folks willing to help who do.

FWIW, most of what I know here is just the product of bashing my head against the wall for extended periods of time. 😅
(not that I recommend much less insinuate you should do the same, but it can be done ;-))

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.

3 participants