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

drivers/pcf857x: Move compile time check to compilation unit #19924

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

maribu
Copy link
Member

@maribu maribu commented Sep 15, 2023

Contribution description

This allows including the header without using the module. Obviously, calls to the functions provided by the header won't like without using the module. But including the header can still be useful for e.g.:

if (IS_USED(MODULE_PCF857x)) {
    /* make use of the module */
}

In the above example all calls to pcf857x functions would be optimized out when the module is not used, full compile checks happen in either case.

Testing procedure

  • binaries should not change
  • including the pcf857x header should work without having selected one of the pcf857x variants, if the driver is not actually used
    • when calling any of the functions provided, linking should fail
    • when using the pcf857x module without any variant, compiling should still fail with a message indicating that (at least) one of the pcf857x needs to be selected

Issues/PRs references

None

This allows including the header without using the module. Obviously,
calls to the functions provided by the header won't like without using
the module. But including the header can still be useful for e.g.:

    if (IS_USED(MODULE_PCF857x)) {
        /* make use of the module */
    }

In the above example all calls to pcf857x functions would be optimized
out when the module is not used, full compile checks happen in either
case.
@maribu maribu added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 15, 2023
@maribu maribu requested a review from gschorcht September 15, 2023 10:56
@github-actions github-actions bot added the Area: drivers Area: Device drivers label Sep 15, 2023
Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

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

Makes sense.

@gschorcht gschorcht added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Sep 15, 2023
@riot-ci
Copy link

riot-ci commented Sep 15, 2023

Murdock results

✔️ PASSED

d0fccdb drivers/pcf857x: Move compile time check to compilation unit

Success Failures Total Runtime
7937 0 7937 14m:46s

Artifacts

@gschorcht gschorcht added the CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train label Sep 15, 2023
@maribu
Copy link
Member Author

maribu commented Sep 15, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 15, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 9609a49 into RIOT-OS:master Sep 15, 2023
28 checks passed
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants