Hi Axel,
I hope you do not take it personal that the PR got rejected. It can be
difficult sometimes to align all directions.
Indeed the PR made al lot of changes only to allow an automated tool to
do its job. I'm not familiar with SAL myself but I agree with Luca the
resulting code is not desirable. Is there anyway SAL can be included as
a test without changing all headers? Did you update these headers by
hand by the way or is this also automated?
Rg,
Arnaud
On 24/10/2023 19:38, Axel R. wrote:
Yep the PR is up for review. It was indeed a trivial issue, there was
a file under packaging/ that needed the new file listed and the Linux
builds started working.
I have one more trivial issue where a file locally formatted with
clang-format -I --style=file doesn’t pass the formatting test on the
online build.
For some reason the local convention appears to mandate spaces before
opening parentheses as in MACRO (x) or __attribute (x) which is not
aligned with most C and C++ formatting rules of the world, where a
space precedes a parenthesis only after language keywords.
The existing code isn’t conforming strictly to the .clang-format file
that is checked-in with the project. Some of the existing files change
if I run clang-format on them with that formatting file, and spaces
get added before some of the few existing parentheses that don’t have
one already (e.g. __declspec(dllimport) in zmq.h)
The clang-* build targets created by CMake on Windows fail because
they call chmod for some reason, so I suspect a new .clang-format file
does not get created? But why is it needed to set write attributes on
existing files during builds? And why any build artefact was
checked-in (if that is the case that .clang-format is supposed to be
generated)
There should not be so much frictions around minor formatting
discrepancies. I think this nonstandard spacing policy should be
abrogated.
“Put a space before an open parenthesis only in control flow
statements, but not in normal function call expressions and
function-like macros.” https://llvm.org/docs/CodingStandards.html
This is aligned with most formatting conventions back to the original
K&R and I think this project should return to that instead of standing
out in a nonstandard way, and the formatting rules file should be
checked-in and not generated as it does not work on all platforms
today (unless there is a compelling reason to create that file during
builds?)
Thoughts?
—Axel
Sent from my iPhone
On Oct 24, 2023, at 04:05, Arnaud Loonstra <[email protected]> wrote:
Seems a trivial problem. Have you created the PR already?
On 22/10/2023 10:04, Axel R. wrote:
Hi, I'm trying to submit a PR, but the Linux builds fail. I added a
new include\zmq_sal.h next to include\zmq.h and #included it as
"zmq_sal.h" from zmq.h, which builds locally on Windows as well as
on Github. However, the Linux builds fails
("../../src/../include/zmq.h:34:10: fatal error: zmq_sal.h: No such
file or directory") I tried #include <zmq_sal.h> to no avail. Any
idea? How was zmq_utils.h included?
_______________________________________________
zeromq-dev mailing list
[email protected]
https://lists.zeromq.org/mailman/listinfo/zeromq-dev
_______________________________________________
zeromq-dev mailing list
[email protected]
https://lists.zeromq.org/mailman/listinfo/zeromq-dev
_______________________________________________
zeromq-dev mailing list
[email protected]
https://lists.zeromq.org/mailman/listinfo/zeromq-dev
_______________________________________________
zeromq-dev mailing list
[email protected]
https://lists.zeromq.org/mailman/listinfo/zeromq-dev
