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

fix: add module name #54

Open
wants to merge 1 commit into
base: zephyr
Choose a base branch
from
Open

Conversation

Jappie3
Copy link

@Jappie3 Jappie3 commented Apr 17, 2024

This MR adds the module name in zephyr/module.yml, as is stated in the official documentation:

Each Zephyr module is given a name by which it can be referred to in the build system.
The name should be specified in the zephyr/module.yml file. This will ensure the module name is not changeable through user-defined directory names or west manifest files.

Copy link
Collaborator

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. There should be a single commit and no merge commits as part of normal PRs (so basically the second commit shouldn't be there).

@Jappie3
Copy link
Author

Jappie3 commented Feb 13, 2025

done

Copy link
Collaborator

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

Please add some of the PR description to the commit message for context. Also, the commit title should rather follow the structure <area touched>: <what is done> so e.g. zephyr/module.yml: add module name

@Jappie3
Copy link
Author

Jappie3 commented Feb 14, 2025

something like this?

The module name is now explicitly set in module.yml, this ensures that
building with e.g. cmake does not fail when the directory name is
different (default name if unset is directory name).
Copy link
Collaborator

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants