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

Makefile.include: remove functionality to build with another version. #10543

Merged
merged 1 commit into from
Jan 17, 2019

Conversation

jcarrano
Copy link
Contributor

@jcarrano jcarrano commented Dec 4, 2018

Contribution description

Makefile.include contained logic to clone a different RIOT version and build the application against it. This can be easily achieved by making a git worktree and overriding RIOTBASE, without adding any complexity to our makefiles. Therefore, the feature is being removed.

Testing procedure

CI will test that nothing got broken.

Makefile.include contained logic to clone a different RIOT version and
build the application against it. This can be easily achieved by making
a git worktree and overriding RIOTBASE, without adding any complexity
to our makefiles. Therefore, the feature is being removed.
@jcarrano jcarrano added 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 Dec 4, 2018
@jcarrano jcarrano requested review from cladmi and jnohlgard December 4, 2018 15:20
@cladmi cladmi added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Dec 5, 2018
@cladmi
Copy link
Contributor

cladmi commented Dec 5, 2018

I also planned to remove it for some time, it introduces a specific branch in the main Makefile.include for something I would not want included in there. Removing it will also simplify cleaning other things.

Maybe we should put a deprecation error for one full release to see if somebody complains.

@cladmi cladmi 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 Jan 17, 2019
@cladmi
Copy link
Contributor

cladmi commented Jan 17, 2019

We do not maintain compatibility with previous versions, even build with docker cannot work, so I do not like having something making think it is the case.

If RIOT_VERSION is now unused, it could even replace the RIOT_VERSION_OVERRIDE. But it would be another PR/discussion.

Copy link
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

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

ACK, I wanted to remove it too as it confused me a lot when setting RIOT_VERSION instead of RIOT_VERSION_OVERRIDE.

There was no objections until now. Also it can easily be reverted/proposed differently if someone complains.

We can merge when murdock is happy.

@cladmi cladmi added this to the Release 2019.01 milestone Jan 17, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good and would be great to have this for the release.

ACK and go

@aabadie aabadie merged commit bb97c05 into RIOT-OS:master Jan 17, 2019
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 1, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 1, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 5, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 8, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 18, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 22, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 24, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 25, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.

When building with `RIOT_CI_BUILD=1` this also now prevents calling
`git describe` and `git rev-parse` which saves around 0.1 seconds
per execution on my machine.
cladmi added a commit to cladmi/RIOT that referenced this pull request Jul 29, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.
cladmi added a commit to cladmi/RIOT that referenced this pull request Aug 12, 2019
RIOT_VERSION is not used for building a specific RIOT version anymore
since RIOT-OS#10543. The variable can now be used directly.
Add deprecation warning.
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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. 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