Skip to content

Bluetooth: testlib: Fix includes (IWYU) #88673

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 1 commit into from
Apr 18, 2025

Conversation

alwa-nordic
Copy link
Collaborator

This commit fixes all includes in testlib according to IWYU rules and sorts all includes in ascending order by name.

@alwa-nordic alwa-nordic requested a review from Copilot April 15, 2025 15:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

@alwa-nordic
Copy link
Collaborator Author

@aescolar
image

Looks like bsim tests were skipped for this. They probably shouldn't. Do we need to adjust the triggers?

aescolar
aescolar previously approved these changes Apr 15, 2025
@aescolar
Copy link
Member

aescolar commented Apr 15, 2025

Looks like bsim tests were skipped for this. They probably shouldn't. Do we need to adjust the triggers?

@alwa-nordic yep. Thanks for pointing it out
#88677

jhedberg
jhedberg previously approved these changes Apr 15, 2025
kartben
kartben previously approved these changes Apr 15, 2025
theob-pro
theob-pro previously approved these changes Apr 16, 2025
Thalley
Thalley previously approved these changes Apr 16, 2025
@@ -2,18 +2,22 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <autoconf.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

autoconf.h is a funny include, because of the way it's included. My Clang-tidy finds it under zephyr/autoconf.h after compilation, but it can also be included like this.

I don't have a opinion of whether to use one or the other, but we probably should be somewhat consistent

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like zephyr/autoconf.h is the preferred way.

zephyr/Kconfig.zephyr

Lines 1105 to 1116 in e1a0350

config LEGACY_GENERATED_INCLUDE_PATH
bool "Legacy include path for generated headers"
default y
help
Allow applications and libraries to use the Zephyr legacy include
path for the generated headers which does not use the `zephyr/` prefix.
From now on, i.e., the preferred way to include the `version.h` header is to
use <zephyr/version.h>, this Kconfig is currently enabled by default so that
user applications won't immediately fail to compile.
This Kconfig will be deprecated and eventually removed in the future releases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The -I argument to gcc for include/generated/zephyr is placed before include/generated on the command line in the compilation database. That's probably why my clangd picked that one up.

This commit fixes all includes in testlib according to IWYU rules and
sorts all includes in ascending order by name.

Signed-off-by: Aleksander Wasaznik <[email protected]>
@alwa-nordic
Copy link
Collaborator Author

Fixed the autoconf includes.

@kartben kartben merged commit a71cadf into zephyrproject-rtos:main Apr 18, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants