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

Style Requirements Seem to Be Inconsistent with Uncrustify Configuration #41281

Closed
catch-twenty-two opened this issue Dec 17, 2021 · 4 comments
Closed
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale

Comments

@catch-twenty-two
Copy link
Member

There seem to be an inconsistency with the required stylistic formatting outlined in the style guide and the formatting settings for Uncrustify. This makes it difficult for new code submissions to conform to what is outlined in the contribution guide and what is expected from the community.

Looks good to me. I'd honestly suggest filing an issue on uncrustify and the docs here if you ran it and it still didn't conform. The docs suggest using it, I've done just that as well, and yet folks still note that it doesn't conform to the style guide which is I agree a confusing thing to run into. Especially as a new contributor.

Agreed, we are preparing to submit more drivers and updates to drivers, I'd hate for things to blocked because of stylistic issues that can't be address without multiple updates to prs.

I agree that it's annoying, and unfortunately tooling like uncrustify seem to be outdated or not working as expected (there are some compliance issues that are likely related to that). You may try with clang-format, from my experience it is not perfect but does a decent job.

Originally posted by @gmarull in #37749 (comment)

@dkalowsk dkalowsk added area: Code Style bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug labels Jan 11, 2022
@nashif nashif assigned gmarull and unassigned nashif Feb 10, 2022
@nordicjm
Copy link
Collaborator

This also includes the clang-format file. An example, file contains:

const struct hash_checksum_mgmt_group *hash_checksum_mgmt_find_handler(const char *name)
{
        /* Find the group with the specified group name */
        struct hash_checksum_mgmt_group *group = NULL;
        sys_snode_t *snp, *sns;

        SYS_SLIST_FOR_EACH_NODE_SAFE(&hash_checksum_mgmt_group_list, snp, sns) {
                struct hash_checksum_mgmt_group *loop_group =

Note that there is no space between SYS_SLIST_FOR_EACH_NODE_SAFE and the bracket, however, running this file through clang-format with the zephyr .clang-format file puts a space between them, then the PR fails because there is a space where there should not be a space.

@gmarull
Copy link
Member

gmarull commented Apr 16, 2022

@lairdjm FYI #41307 (I'll pick this again when I come back from paternity leave in a few weeks)

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Jun 16, 2022
@gmarull
Copy link
Member

gmarull commented Jun 16, 2022

We have updated clang-format and removed uncrustify in #41307

@gmarull gmarull closed this as completed Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

No branches or pull requests

5 participants