Skip to content

Conversation

mrjimenez
Copy link
Contributor

Slip is naturally promiscuous, so this patch does nothing but acknowledge that. Promiscuous mode in slip is important to allow the interface to be added to a bridge.

Copy link

github-actions bot commented Oct 9, 2025

Hello @mrjimenez, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

jukkar
jukkar previously approved these changes Oct 9, 2025
@mrjimenez
Copy link
Contributor Author

Fixed the issues with code/commit message compliance.

@mrjimenez mrjimenez requested a review from jukkar October 9, 2025 20:11
Copy link
Member

@jukkar jukkar left a comment

Choose a reason for hiding this comment

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

You have mixed changes in the commit. The indentation/style things should be separated into a different commit than the promisc mode changes.

Comment on lines 59 to 60
ETH_NET_DEVICE_INIT(slip, CONFIG_SLIP_DRV_NAME, slip_init, NULL, &slip_context_data, NULL,
CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, &slip_if_api, _SLIP_MTU);
Copy link
Member

Choose a reason for hiding this comment

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

I do not like this change, it looks like you used clang formatter which makes long lines that are just harder to read than the original code. The other format changes look ok (just place into separate commit).

Copy link
Contributor Author

@mrjimenez mrjimenez Oct 11, 2025

Choose a reason for hiding this comment

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

Hi Jukka,

I totally agree with you, really, I think it got much uglier. But since it is my first commit on the project, and I got a "non-conformity" error on the automated tests, I assumed all files should have passed throuhg the project's defined clang-format.

I will do as you said, split the patch and format as before.

Copy link
Member

Choose a reason for hiding this comment

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

The CI reports clang-format issues but they are only suggestions (at least for now) and can be ignored if they do not make sense/look ugly.

This patch just formats the file before the real patch so that
whitespace changes do not mix with the real code changes.

Signed-off-by: Marcelo Roberto Jimenez <[email protected]>
Slip is naturally promiscuous, so this patch does nothing but
acknowledge that. Promiscuous mode in slip is important to allow
the interface to be added to a bridge.

Signed-off-by: Marcelo Roberto Jimenez <[email protected]>
Copy link

@cfriedt cfriedt merged commit 47e7b7b into zephyrproject-rtos:main Oct 13, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants