Skip to content

[stm32] move CMSIS hearder files to common folder #5998

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

Merged
merged 2 commits into from
Jun 4, 2022

Conversation

mysterywolf
Copy link
Member

@mysterywolf mysterywolf commented May 25, 2022

拉取/合并请求描述:(PR description)

[
将CMSIS头文件抽出来 统一放在STM32 libraries文件夹下,防止重复文件一轮叠一轮

已经测试通过无论是直接编译还是dist出来的工程都是没有问题的
]

以下的内容不应该在提交PR时的message修改,修改下述message,PR会被直接关闭。请在提交PR后,浏览器查看PR并对以下检查项逐项check,没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 本拉取/合并请求代码是高质量的 Code in this PR is of high quality
  • 本拉取/合并使用formatting等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

@mysterywolf mysterywolf changed the title Finsh [stm32] move CMSIS hearder files to common folder May 25, 2022
@Guozhanxin
Copy link
Member

如果没有特别大的提升,不建议这么修改,之前只是移除一部分cmsis的文件还好,这个pr相当于把cmsis分成了两部分,一部分系统统一管理,一部分各个库管理,还不在一个地方。容易给后期更新、维护造成困扰

1 similar comment
@Guozhanxin
Copy link
Member

如果没有特别大的提升,不建议这么修改,之前只是移除一部分cmsis的文件还好,这个pr相当于把cmsis分成了两部分,一部分系统统一管理,一部分各个库管理,还不在一个地方。容易给后期更新、维护造成困扰

@mysterywolf
Copy link
Member Author

现在有一些bsp一个BSP就含着一个CMSIS,CMSIS越来越多了。

@mysterywolf
Copy link
Member Author

mysterywolf commented May 26, 2022

后期维护不会出现过多困扰的,写一个说明文档就好了。现在为了维护偷懒 导致仓库里存了大量了无用、重复的文件
而且STM32的文件结构会误导其他厂商BSP的提交

@mysterywolf mysterywolf requested a review from liukangcc May 30, 2022 22:44
@liukangcc liukangcc added the +1 Agree +1 label May 31, 2022
@supperthomas
Copy link
Member

supperthomas commented Jun 3, 2022

这个改动非常好,我之前也觉得很多头文件很大(几M多),也重复。
就是提醒一下,我之前好像遇到过因为这个CMSIS.h文件版本不一样(文件名一样,内容不一样),导致代码运行逻辑不一样的。不知道这里的版本到时候好不好控制,也不知道这样改了之后,有些STM32代码运行逻辑会不会有所不同。
改动方向支持的,建议要加强版本维护。或者是否考虑软件包来管理这些文件
+1

@supperthomas supperthomas added BSP: STM32 BSP related with ST/STM32 wait_+2 wait for "+2" to confirm labels Jun 3, 2022
@mysterywolf mysterywolf added +2 Agree +2 and removed wait_+2 wait for "+2" to confirm labels Jun 3, 2022
@BernardXiong BernardXiong merged commit b197b50 into RT-Thread:master Jun 4, 2022
@mysterywolf mysterywolf deleted the finsh branch June 4, 2022 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BSP: STM32 BSP related with ST/STM32 important +1 Agree +1 +2 Agree +2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants