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

zephyr: module.yml: Add missing module name #227

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

erwango
Copy link
Member

@erwango erwango commented Aug 22, 2024

Field name is missing in module.yml file.
Fix this.

@erwango erwango requested review from FRASTM and gautierg-st August 22, 2024 13:22
@erwango
Copy link
Member Author

erwango commented Aug 22, 2024

@jhedberg FYI. Thanks for the heads up.

@jhedberg
Copy link
Member

@erwango btw, this changes the command used for fetching blobs from west blobs fetch stm32 to west blobs fetch hal_stm32, so if you have any documentation with such references it may need to be updated.

@erwango
Copy link
Member Author

erwango commented Aug 23, 2024

@erwango btw, this changes the command used for fetching blobs from west blobs fetch stm32 to west blobs fetch hal_stm32, so if you have any documentation with such references it may need to be updated.

Valuable information indeed. Thanks, I'll check this.

@erwango
Copy link
Member Author

erwango commented Aug 23, 2024

@erwango btw, this changes the command used for fetching blobs from west blobs fetch stm32 to west blobs fetch hal_stm32, so if you have any documentation with such references it may need to be updated.

PR updated to keep the ability to keep the existing command west blobs fetch stm32 functional.

Copy link
Member

@jhedberg jhedberg left a comment

Choose a reason for hiding this comment

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

@erwango btw, this changes the command used for fetching blobs from west blobs fetch stm32 to west blobs fetch hal_stm32, so if you have any documentation with such references it may need to be updated.

PR updated to keep the ability to keep the existing command west blobs fetch stm32 functional.

@erwango the reason I pointed out the difference was just to update documentation. I still think that hal_stm32 is the correct name, since then it's consistent with how all other Zephyr HAL modules are defined and then it's also equal to the module name as it is defined in west.yml:

https://github.com/zephyrproject-rtos/zephyr/blob/143b14bb4fb779437fdeb8369e42fbb81db4a5fd/west.yml#L235

If you just call it stm32 it's equally (if not more) likely that people will be confused when they are not able to fetch blobs the same way as they do for other modules. Furthermore, the name hal_stm32 would be consistent with how the various CMake and Kconfig variables are defined (I suspect those get created based on west.yml).

This used to be an even bigger problem before: zephyrproject-rtos/zephyr#73901, however even now I think it's important to be consistent and predictable between modules. We used to have the same bug in hal_silabs, but fixed it with zephyrproject-rtos/hal_silabs@a09dd1b

@erwango
Copy link
Member Author

erwango commented Aug 26, 2024

@jhedberg I was just wanting to avoid impacting users for vain reason, but since you gave me valid ones, I'll do the change to hal_stm32.

Field `name` is missing in module.yml file.
Fix this, using the canonical name as used by other hals.

Signed-off-by: Erwan Gouriou <erwan.gouriou@st.com>
@erwango erwango removed the DNM label Aug 27, 2024
@erwango erwango merged commit 4c1adf8 into zephyrproject-rtos:main Aug 27, 2024
4 checks passed
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