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

Migrate Travis to the new travis-ci.com platform #2538

Merged
merged 3 commits into from
Oct 12, 2018
Merged

Migrate Travis to the new travis-ci.com platform #2538

merged 3 commits into from
Oct 12, 2018

Conversation

SergioRAgostinho
Copy link
Member

@SergioRAgostinho SergioRAgostinho commented Oct 10, 2018 •
edited
Loading

  • upgrade distribution to xenial
  • merge test jobs into a single one
  • merge lib, examples, tools, apps jobs into a single one
  • docs are now also built in PRs

Closes #2531

@jspricke jspricke changed the title Finish migration to tracis-ci.com [WIP don't merge] Finish migration to tracis-ci.com Oct 10, 2018
@jspricke
Copy link
Member

I guess you are still testing this, so I changed the title ;).

jspricke
jspricke reviewed Oct 10, 2018
View reviewed changes
.travis.yml Outdated
@@ -8,22 +9,22 @@ addons:
- sourceline: 'ppa:v-launchpad-jochen-sprickerhof-de/pcl'
Copy link
Member

Choose a reason for hiding this comment

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

I guess you can remove this ;).

.travis.sh Show resolved Hide resolved
jspricke
jspricke reviewed Oct 10, 2018
View reviewed changes
.travis.yml Show resolved Hide resolved
jspricke
jspricke reviewed Oct 10, 2018
View reviewed changes
.travis.yml Outdated
@@ -28,6 +26,8 @@ addons:
- libglew-dev
- libopenni-dev
- python3-pip
- python3-sphinxcontrib.doxylink
Copy link
Member

Choose a reason for hiding this comment

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

Wow, just found that Ubuntu only added this in artful, even though it's in Debian since 2016.. So you have to go back to pip, sorry.

@SergioRAgostinho SergioRAgostinho changed the title [WIP don't merge] Finish migration to tracis-ci.com Finish migration to tracis-ci.com Oct 10, 2018
@SergioRAgostinho SergioRAgostinho changed the title Finish migration to tracis-ci.com Migrate Travis to the new travis-ci.com platform Oct 10, 2018
@SergioRAgostinho
Copy link
Member Author

Ready for the final review. We finally have the testing failure behavior. 👍

I would also like to disable the travis-ci.org webhook. They're both providing the same check now.

@SergioRAgostinho SergioRAgostinho modified the milestone: pcl-1.9.0 Oct 11, 2018
@taketwo
Copy link
Member

taketwo commented Oct 11, 2018

A few proposals:

  1. What about explicitly disabling the "doc" job for pull requests? This can be done by adding the following line to the job definition:
      if: type != pull_request
  1. While we are at this, we may add
git:
  depth: 1

since we only need the current state of the repository.

  1. Give names to the jobs, e.g.:
      name: "Build"

@jspricke
Copy link
Member

Looks awesome, fine with me to merge now or after the tests are fixed.

@SergioRAgostinho
Copy link
Member Author

SergioRAgostinho commented Oct 11, 2018 •
edited
Loading

A few proposals:

  1. What about explicitly disabling the "doc" job for pull requests? This can be done by adding the following line to the job definition:
      if: type != pull_request

I'm conflicted about this one. Adopting this means will mean that every now and then we'll have build reports failing directly on the master branch. Allowing the job in the PRs allow us to detect things earlier and avoids the usual edits to .travis.sh just to be able see things failing.

  1. While we are at this, we may add
git:
  depth: 1

Fully agree.

since we only need the current state of the repository.

  1. Give names to the jobs, e.g.:
      name: "Build"

It's finally implemented 🙌 . Fully endorse

Looks awesome, fine with me to merge now or after the tests are fixed.

The tests are being fixed in different PRs. I will however adopt suggestions 2. and 3. from Sergey, and potentially (or not) 1., depending on the agreement we reach.

@SergioRAgostinho
Migrate Travis to the new travis-ci.com platform.
ffa7acc 
- upgrade distribution to xenial
- merge test jobs into a single one
- merge lib, examples, tools, apps jobs into a single one
- docs are now also built in PRs
@SergioRAgostinho
Update the badge link to the new domain
622e420
@taketwo
Copy link
Member

taketwo commented Oct 11, 2018

Adopting this means will mean that every now and then we'll have build reports failing directly on the master branch.

You mean for pull requests that change something in Travis config? OK, makes sense.

@SergioRAgostinho
Copy link
Member Author

Every now and then our docs job breaks usually due to upstream. This would allow us to catch them early on. This is what I meant to say.

If someone wants to change things to the docs setup they can't directly. The process always require an extra commit/squash to disable/restore the "run job on PRs" setting. It's minimal but cumbersome.

Imagine one day we have a user which decides to fix all the doxigen warnings! He will now have an easier time doing it. ^^

@taketwo
Copy link
Member

taketwo commented Oct 11, 2018

@jspricke you mentioned in #2531 (comment) that we now have 100 minutes. Where did you get this information? The latest build timed out at 1 hour 10 minutes...

@SergioRAgostinho
Copy link
Member Author

SergioRAgostinho commented Oct 11, 2018 •
edited
Loading

The 100 minutes are at travis-ci.com. The .org is still running under the same limits.

@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Oct 12, 2018
@SergioRAgostinho
Copy link
Member Author

I need this merged please.

@jspricke jspricke merged commit b13d5f0 into PointCloudLibrary:master Oct 12, 2018
@SergioRAgostinho
Copy link
Member Author

I will now disable the travis-ci.org webhook.

@SergioRAgostinho SergioRAgostinho deleted the travis branch October 12, 2018 08:33
@jspricke
Copy link
Member

Did so already :P

@taketwo
Copy link
Member

taketwo commented Oct 12, 2018

Doc job fails: https://travis-ci.com/PointCloudLibrary/pcl/jobs/151400719

Do we maybe need to regenerate security tokens that allow doc job on Travis to push to the documentation repository?

@SergioRAgostinho
Copy link
Member Author

I'm also a little worried about this last line

fatal: Authentication failed for 'https://github.com/PointCloudLibrary/documentation.git/

the endpoint doesn't seem to have been updated...

@SergioRAgostinho SergioRAgostinho mentioned this pull request Oct 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taketwo taketwo taketwo approved these changes

@jspricke jspricke jspricke approved these changes

Assignees
No one assigned
Labels
module: ci
Projects
None yet
Milestone
pcl-1.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@SergioRAgostinho @jspricke @taketwo