-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
CI: build examples for additional code verification #7922
Conversation
Is Circle CI actually running for PRs? I can't find it in the listing below or am I just blind? @bagder |
Quoting @jay from #7591 (comment) for further discussion:
Thanks. I just checked and found that
So the error message mentions I think something is off with how the typedef's are done if used from the examples. This looks like a mismatch to me. |
Another interesting find: the error message does not appear in the Cirrus Windows builds which are using the same Docker images as the Azure Windows builds, therefore use the identical toolchain. The only difference is that the Cirrus Windows builds are release and not debug builds. |
This looks like #6079, which was solved by ignoring |
319c426
to
41c32c4
Compare
Thanks for digging this up @jay. Let's see what happens if I put the same workaround into the examples, although I am still wondering why the Cirrus CI builds are not affected without it. It also looks like this is mostly affecting external code, because curl internal code mostly uses the |
Regarding the follow up PR mentioned above. I just created a draft one, since the pragma is now obsolete due to #6535. |
0c429a8
to
f784edc
Compare
f784edc
to
b5c1b67
Compare
b5c1b67
to
a002951
Compare
a002951
to
f5b4298
Compare
f5b4298
to
7b4c3ec
Compare
@bagder I don't have the time now, but you may wanna investigate this error that shows up in AppVeyor for master and this PR:
|
tool_operate.c(889) : warning C4701: potentially uninitialized local variable 'per' use Follow-up to cc71d35 Reported-by: Marc Hörsken Bug: #7922 (comment)
tool_operate.c(889) : warning C4701: potentially uninitialized local variable 'per' use Follow-up to cc71d35 Reported-by: Marc Hörsken Bug: #7922 (comment)
tool_operate.c(889) : warning C4701: potentially uninitialized local variable 'per' use Follow-up to cc71d35 Reported-by: Marc Hörsken Bug: #7922 (comment) Closes #7971
7b4c3ec
to
f82e981
Compare
f82e981
to
d1a2c88
Compare
@bagder please confirm with another approval if you are fine with adding the pragma to the relevant examples, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the added pragma lines are unfortunate since the examples are meant to contain as little "extra fluff" as possible. I assume you haven't figured out any other way to work around the problems you get without them? Can we remove/change the printf lines instead?
d1a2c88
to
cd64f97
Compare
@bagder I just tried to update the relevant |
cd64f97
to
73a9183
Compare
Some CIs already build them, let's do it on more of them.
Follow up to #7690
Replaces #7591