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

oonf_api: use RIOT_VERSION as platform identifier for RIOT #2805

Merged
merged 1 commit into from
Apr 19, 2015

Conversation

benpicco
Copy link
Contributor

Some third-party code my come with #ifdefs to conditionally compile some parts for RIOT, while maintaining the possibility to compile for Linux, Windows, etc.

This makes oonf_api check if RIOT_VERSION is defined for RIOT specific codepaths.

fixes #2772

@Kijewski
Copy link
Contributor

Strong dislike. We have RIOT_BOARD, RIOT_CPU, RIOT_MCU and RIOT_VERSION. I'd prefer #ifdef RIOT_BOARD.

@OlegHahm OlegHahm self-assigned this Apr 14, 2015
@OlegHahm OlegHahm added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Apr 14, 2015
@OlegHahm
Copy link
Member

Independent from the question if we need this (see #2772), I would argue that a platform identifying macro is something completely else than the RIOT internal defines specifying the board or CPU.

@kaspar030
Copy link
Contributor

Strong dislike. We have RIOT_BOARD, RIOT_CPU, RIOT_MCU and RIOT_VERSION. I'd prefer #ifdef RIOT_BOARD.

I tend to agree here, those defines should be enough.

I would argue that a platform identifying macro is something completely else than the RIOT internal defines specifying the board or CPU.

That's why I'd go for #ifdef RIOT_VERSION.

@benpicco benpicco force-pushed the platform_identify branch from 5770cc9 to fb29fe3 Compare April 14, 2015 17:21
@benpicco
Copy link
Contributor Author

That's why I'd go for #ifdef RIOT_VERSION.

Ok.

@benpicco benpicco force-pushed the platform_identify branch from fb29fe3 to a054a98 Compare April 14, 2015 17:22
@haukepetersen
Copy link
Contributor

Agree with @Kijewski and @kaspar030, RIOT_VERSION just do just fine.

@OlegHahm
Copy link
Member

Sounds good to me. ACK if Travis is fine.

@LudwigKnuepfer
Copy link
Member

Please change the commit message to use the module as a prefix. (Compare git log)

@OlegHahm
Copy link
Member

kicked Travis

@OlegHahm
Copy link
Member

For the record: Travis is green - if you just change the commit message, we're ready to go.

@benpicco benpicco changed the title add compile time platform identifier for RIOT oonf_api: use RIOT_VERSION as platform identifier for RIOT Apr 18, 2015
@benpicco benpicco force-pushed the platform_identify branch from a054a98 to 7a2b705 Compare April 18, 2015 22:22
@benpicco
Copy link
Contributor Author

Am Wed, 15 Apr 2015 22:52:58 -0700
schrieb Ludwig Ortmann notifications@github.com:

Please change the commit message to use the module as a prefix.

done

@OlegHahm
Copy link
Member

and go

OlegHahm added a commit that referenced this pull request Apr 19, 2015
oonf_api: use RIOT_VERSION as platform identifier for RIOT
@OlegHahm OlegHahm merged commit 279fd09 into RIOT-OS:master Apr 19, 2015
@benpicco benpicco deleted the platform_identify branch April 22, 2015 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

oonf_api requires undocumented define
6 participants