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

native: only export NATIVEINCLUDES in vars.inc.mk #13720

Merged
merged 3 commits into from
Apr 7, 2020

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Mar 26, 2020

Contribution description

This PR only export NATIVEINCLUDES from vars.inc.mk and remove the use of export in other places. A static check is added in the build system check script.

I tried to completely get rid of the export but then the build fails unless if undoing #8940. So I would keep the export for now.

Testing procedure

  • A green Murdock should be enough

Issues/PRs references

Related to #10850

@aabadie aabadie added Platform: native Platform: This PR/issue effects the native platform Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 26, 2020
@aabadie aabadie requested a review from fjmolinas March 26, 2020 10:03
@fjmolinas
Copy link
Contributor

Did you test on valgrind?

@aabadie
Copy link
Contributor Author

aabadie commented Apr 1, 2020

Did you test on valgrind?

Yes I had the same output. I don't if both were valid or not though.

@leandrolanzieri leandrolanzieri self-assigned this Apr 7, 2020
@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 7, 2020
@leandrolanzieri leandrolanzieri added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 7, 2020
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Changes look good. ACK.

@leandrolanzieri leandrolanzieri merged commit d60295d into RIOT-OS:master Apr 7, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Apr 7, 2020
@aabadie aabadie deleted the pr/native_no_export branch June 24, 2020 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants