-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
google-cloud-cpp/all: disable werror #21808
Conversation
Tony Perrie seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
🤖 Beep Boop! This pull request is making changes to 'recipes/boost//'. 👋 @grafikrobot @Hopobcn @jwillikers you might be interested. 😉 |
73e7ffd
to
97c7d89
Compare
This comment has been minimized.
This comment has been minimized.
@hoyhoy Hello! Please, describe better your situation. This project is built in Conan Center and does not break because of -Werror flag. Please, use the issue form for Bug report in https://github.com/conan-io/conan-center-index/issues/ to give us all information needed. |
@uilianries I ended up having to fork the google-cloud-cpp v1.X recipe. It's not conan2 compatible. In fact, we had to fork nearly all of our recipes because nearly all of the conan center recipes have bugs, don't build on s/390, don't build on clang17 with warnings enabled, don't build on windows, and so on. I was going to try to port some of this upstream, but it takes three to four weeks to get PRs reviewed on conan-center. |
@uilianries And I don't see why you'd want to have Werror on for a production build. These C++ warnings are all over the place depending on what compiler and OS you happen to be using. Basically, what you just told me is, "WORKS ON MY MACHINE!". Ok, great, but did you test it s/390, windows, clang 17, gcc 13.2, macos arm, gcc 8.5, AIX, and MSVC? Because I have, and I can 100% assure Werror does not work on all of those platforms. |
This comment has been minimized.
This comment has been minimized.
@hoyhoy Your PR can be genuine, but when I asked to open the issue is because your PR has no issue associated and was not clear enough about your error. It's true we can not test all users configurations in the CI, but it does not block from merging fixes, since it's well documented, including with a build log where we consult, not only a prove, in case needing to revisit it in the future. There is a second problem regarding this recipe: It's not ported to Conan 2.x yet, and the CI requires it as a mandatory rule. We will need to open a separated PR to update his recipe. Anyway, we could merge it manually in case not passing. Also, could you please check the account reserved to |
An issue could be opened, but this fix looks quite trivial to me, and we all know that warnings as errors must not be enabled in conancenter recipes. FYI, warnings as errors are disabled by default in google-cloud-cpp since googleapis/google-cloud-cpp#10819 (2.8.0). By the way, Anyway google-cloud-cpp < 2.x has to be ported to conan v2 first. |
conan v2 migration: #22591 |
Conan v1 pipeline ❌Failure in build 1 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. Conan v2 pipeline ❌
The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping See details:Failure in build 1 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
@hoyhoy there are commits here not signing the CLA, could you please verify it? |
Yeah, it was easier just to fork the recipe. I signed the CLA three times. I think there was some issue with my corporate email being in my .gitconfig previously, but I thought I fixed it. Eventually I just gave up. |
Ok, we were trying to force-merge this by hand, we can bypass the CI, but we cannot bypass the CLA. |
V1 is EOL anyway. We have a library still using the cloud storage component that we still haven't upgraded. |
Closing in favour of #22591 which already ahs done the Conan v2 support work |
google-cloud-cpp/all
There are various problems with warning as errors.
I'm already ignoring....
This is the latest issues...