search

Kawasaki-Robotics / khi_robot

ROS KHI robot meta-package
http://wiki.ros.org/khi_robot
BSD 3-Clause "New" or "Revised" License
55 stars 28 forks source link

update travis.yml #6

Closed k-okada closed 5 years ago

k-okada commented 5 years ago

Created travis.yml using

Please activate GitHub - TravisCI integration to enable this test https://github.com/apps/travis-ci

k-okada commented 5 years ago

@d-nakamichi can we enable TravsiCI ?

Go through the instructution here https://qiita.com/hoshimado/items/4090d8e64beb8a7f95e1#travis-ci%E3%82%A2%E3%82%AB%E3%82%A6%E3%83%B3%E3%83%88%E4%BD%9C%E6%88%90%E3%81%A8%E5%AF%BE%E8%B1%A1%E3%81%AEgithub%E3%83%AA%E3%83%9D%E3%82%B8%E3%83%88%E3%83%AA%E3%82%92%E7%B4%90%E4%BB%98%E3%81%91 Until “これで、Travis CI側の設定は完了。”

d-nakamichi commented 5 years ago

Thanks for your help! Now I activated GitHub - TravisCI integration.

gavanderhoorn commented 5 years ago

@k-okada: did you have any particular reason not to use industrial_ci? It's not a panacea, but leads to very readable and simple .travis.yml files (see this one as an example).

Edit: test build with industrial_ci: test build (takes approx 10-ish minutes per build config matrix row).

gavanderhoorn commented 5 years ago

@k-okada: it appears you merged an older version of https://github.com/Kawasaki-Robotics/khi_robot/compare/master...gavanderhoorn:pkging_updates.

k-okada commented 5 years ago

@gavanderhoorn yes, i know. I just want travis to answer

@k-okada: could you verify this also works for you?

question.

Please wait at most 16 hours, I will finalize this PR and ready to merge and release working version so that we can put khi_robot to the this week's sync.

gavanderhoorn commented 5 years ago

Your proposed travis config is not working because it also includes an Indigo build. The IKFast plugins cannot build under Indigo.

@gavanderhoorn yes, i know. I just want travis to answer

@k-okada: could you verify this also works for you?

question.

My question was specifically about https://github.com/gavanderhoorn/khi_robot/commit/6641defd47a814ab113ce154b0f55471939aaf75. Your add_travis branch does not contain that commit.

I'm also unsure whether Travis can catch linking issues with dynamic libraries, as there are currently no tests, so no binaries are ever run.

I've run a quick Travis test here with an up-to-date https://github.com/Kawasaki-Robotics/khi_robot/compare/master...gavanderhoorn:pkging_updates and that seems to succeed.

d-nakamichi commented 5 years ago

@k-okada @gavanderhoorn

Which is better? For testing just build/install package, industrial_ci seems better because it can select prerelease or not. Run ROS Prerelease Test

k-okada commented 5 years ago

@d-nakamichi @matsui-hiro @gavanderhoorn Although 2c0a73e did not failed as I expected, but I think this PR is ready to merge and release.

==

@k-okada: did you have any particular reason not to use industrial_ci?

1) ros_buildfarm is less likely to loose maintainer, If ros_buildfarm is orphaned and we have no alternatives, that’s mean the end of ROS. For example, every time we have new ROS distro or new Ubuntu version, we can assume it is OSRF jobs to update the ros_buildfirm to support them. But as for industrial_ci, someone have to do that. (In my opinion, if ROS-I have the resource to maintain industrial_ci that effort should goes to maintain image_pipeline, rqt, ros-drivers,,, and so on...)

2) My understanding of industrial_ci is customizable, (At least originally, because that is originally coms from our http://github.com/jsk-ros-pkg/jsk_travis ) So when the source tree did not pass the travis we have opetion to customize the CI parts, for example skip some package, run extra scripts and so on...., and it is good compromise between stability vs development speed, for research labs. But for industrial application, stability is much important. So if we do not pass the travis, we should fix source tree. (So ideally, we should run build script, in addition to prerelease, because our intrest is if the source tree is able to run on the buildfirm.)

d-nakamichi commented 5 years ago

OK. khi_robot uses prerelease at the begging. I'll accept this PR. Thanks! @k-okada @gavanderhoorn

gavanderhoorn commented 5 years ago

@d-nakamichi @k-okada: this PR included a lot of mostly my commits. I would have appreciated it if #5 could have been merged instead. Now changes to the packaging and build setup are merged from a PR that is unrelated (ie: one that "update[s] travis.yml"), which is not good maintainership practice.

This cannot be fixed now any more, but in the future please do not do this with my pull requests.

My apologies, the github UI confused me. It seems both PRs were merged.

gavanderhoorn commented 5 years ago

@k-okada: did you have any particular reason not to use industrial_ci?

1. ros_buildfarm is less likely to loose maintainer, If ros_buildfarm is orphaned and we have no alternatives, that’s mean the end of ROS. For example, every time we have new ROS distro or new Ubuntu version, we can assume it is OSRF jobs to update the ros_buildfirm to support them. But as for industrial_ci, someone have to do that.

I don't really follow your argument here.

Are you worried about whether industrial_ci will be maintained in the future?

(In my opinion, if ROS-I have the resource to maintain industrial_ci that effort should goes to maintain image_pipeline, rqt, ros-drivers,,, and so on...)

I guess we don't agree here: ROS-Industrial is not responsible for maintaining all sorts of packages. Resources are limited everywhere, so we have to make choices.

industrial_ci fulfils a direct need of ROS-Industrial members, so it makes sense to spend effort there.

2. My understanding of industrial_ci is customizable, (At least originally, because that is originally coms from our [http://github.com/jsk-ros-pkg/jsk_travis](https://github.com/jsk-ros-pkg/jsk_travis) ) So when the source tree did not pass the travis we have opetion to customize the CI parts, for example skip some package, run extra scripts and so on...., and it is good compromise between stability vs development speed, for research labs. But for industrial application, stability is much important. So if we do not pass the travis, we should fix source tree.  (So ideally, we should run build script, in addition to prerelease, because our intrest is if the source tree is able to run on the buildfirm.)

re: customizable -> for research labs: I don't fully understand your reasoning here. industrial_ci supports running prereleases as well as many other things. Changing .travis.yml for industrial_ci has to go through the same process (ie: a pull request that should be reviewed) as with the current setup.

What you write are maintenance policy / development process concerns. All software is customisable, and I doubt industrial_ci is any more customisable than the current .travis.yml. In either case: no maintainer can just go in and chance CI configuration, unless the repository has been badly configured.

k-okada commented 5 years ago

@gavanderhoorn Then, what is the reason for having industrial_ci? in addition to the prerelease.py ?

gavanderhoorn commented 5 years ago

@gavanderhoorn Then, what is the reason for having industrial_ci? in addition to the prerelease.py ?

I believe it would a bit too much to discuss this in a comment.

I'd refer you to the readme and the rest of the documentation.

For me personally, readability of the travis configuration is a big plus. Compare: this with this (if pinning of industrial_ci is a concern, specify a version here).

Please note that I don't feel strongly about this. If you're happy with what you're using right now then please keep using that :)

k-okada commented 5 years ago

For me personally, readability of the travis configuration is a big plus. Compare: this with this (if pinning of industrial_ci is a concern, specify a version here).

If that is the problem, prepare one script and write curl -sL ttps:// raw.github.com/ros-industrial/industrial_ci/master/run_prerelease.sh | bash in travis.yml

Please note that I don't feel strongly about this.

Yeah, I know.