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.

Sign up for GitHub

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

Jump to bottom

.travis.yml: provide base branch to static tests #10765

Merged
merged 2 commits into from
Jan 15, 2019
Merged

.travis.yml: provide base branch to static tests #10765

merged 2 commits into from
Jan 15, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 14, 2019

Contribution description

In #10759 we noticed that Travis was trying to rebase to master, though the PR was against 2018.10-branch. I suspect the environment was not set properly in #10251. According to the Travis documentation this should be fixed the way I did here.

Testing procedure

Run Travis against master and another base branch.

Issues/PRs references

Discovered in #10759, but unrelated.

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: CI Area: Continuous Integration of RIOT components labels Jan 14, 2019
@miri64 miri64 added this to the Release 2019.01 milestone Jan 14, 2019
@miri64 miri64 changed the title Travis/fix/base branch .travis.yml: hand base branch to static tests Jan 14, 2019
@jcarrano jcarrano added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 14, 2019
jcarrano
jcarrano previously approved these changes Jan 14, 2019
View reviewed changes
Copy link
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

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

Travis is building it, that means it works.

@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

Well we should also check against one of the release branches. Let me do a quick rebase.

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 14, 2019
@miri64 miri64 changed the base branch from master to 2018.10-branch January 14, 2019 11:54
@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

Mh... Travis doesn't know about 2018.10-branch :-/

@miri64 miri64 force-pushed the travis/fix/base-branch branch 3 times, most recently from b5c0e04 to 4a692a4 Compare January 14, 2019 12:25
@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

@cladmi @jcarrano can you please look into it. I need to go ahead with other things right now.

@cladmi
Copy link
Contributor

cladmi commented Jan 14, 2019

I think it comes from the cloning method.
git clone --depth=50 https://github.com/RIOT-OS/RIOT.git RIOT-OS/RIOT

I will check locally.

@cladmi
Copy link
Contributor

cladmi commented Jan 14, 2019

It worked with this method.

@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

(I would rather put it in before and also not make it a squash commit ;-))

@cladmi
Copy link
Contributor

cladmi commented Jan 14, 2019

Yeah, I can change it, I wanted to see if it was working it travis or not.
Do you want me to rewrite the history in your branch or do it yourself ?

@miri64 miri64 changed the title .travis.yml: hand base branch to static tests .travis.yml: provide base branch to static tests Jan 14, 2019
@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

Done it myself :-)

@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019

And travis likes it. Shall I rebase to master again?

@miri64
Copy link
Member Author

miri64 commented Jan 14, 2019 •
edited
Loading

This change probably won't affect PRs against the 2018.10-branch, as the branch still uses the trusty configuration. Since the branch is also checked statically by Murdock, I don't think this matters too much, but now I see this PR more as a preparation for 2019.01-branch ;-)

@jcarrano
Copy link
Contributor

I suggest we print the base branch/base commit in the output of the static tests.

@miri64
Copy link
Member Author

miri64 commented Jan 15, 2019

If you think that helps, please feel free to open a PR for that. Doesn't really has that much to do with this PR.

@cladmi
Copy link
Contributor

cladmi commented Jan 15, 2019

Yes please rebase to master and enable "CI: Ready" at the same time.

@miri64 miri64 changed the base branch from 2018.10-branch to master January 15, 2019 12:29
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 15, 2019
@miri64
Copy link
Member Author

miri64 commented Jan 15, 2019

Done

cladmi
cladmi approved these changes Jan 15, 2019
View reviewed changes
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, agreed with the changes and it worked against 2018.10-branch (even if this one is getting merged in master).

Putting the fetch in before_script looks like the right place for it.

@miri64
Copy link
Member Author

miri64 commented Jan 15, 2019

Without changing anything I force pushed the last commit to re-trigger travis. I think it got confused by the base-branch change.

@cladmi cladmi merged commit 94a89f5 into RIOT-OS:master Jan 15, 2019
@miri64 miri64 deleted the travis/fix/base-branch branch January 15, 2019 13:26
@miri64 miri64 mentioned this pull request Jan 17, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cladmi cladmi cladmi approved these changes

@jcarrano jcarrano jcarrano left review comments

@aabadie aabadie Awaiting requested review from aabadie

Assignees
No one assigned
Labels
Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2019.01
Development

Successfully merging this pull request may close these issues.
3 participants
@miri64 @cladmi @jcarrano