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

update to industrial_ci #100

Closed
wants to merge 2 commits into from
Closed

Conversation

souravran
Copy link

No description provided.

@mathias-luedtke
Copy link
Contributor

yeah, another candidate for ros-industrial/industrial_ci#123
In this case we should fix cob_kinematics to use the build space.

@mathias-luedtke
Copy link
Contributor

@souravran: you can run all tests in a docker-enabled shell with the run_ci script:

  • clone industrial_ci into your workspace
  • rosrun industrial_ci run_ci $PATH_TO_REPO ROS_DISTRO=indigo

* do not write plugins.xml in source space
* added install tags
compiler:
- gcc

notifications:
Copy link
Contributor

@mathias-luedtke mathias-luedtke Feb 15, 2017

Choose a reason for hiding this comment

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

why did you add this?
I don't want to get mails for every failure.
The default works quite fine for me.

Copy link
Author

Choose a reason for hiding this comment

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

I am not aware of the intricacies for now, but if needed then we can roll back. @ipa-fmw

Copy link
Member

Choose a reason for hiding this comment

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

Well, if we remove this...I want it removed in all the other repositories, too
For the sake of consistency!

Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of consistency!

Constistency does not matter in this case, it is just a preference of the maintainer.

However, I just found it that

  email:
    on_success: change
    on_failure: always

is the default (https://docs.travis-ci.com/user/notifications/).
So perhaps it would make sense to set on_failure: change.

Copy link
Member

Choose a reason for hiding this comment

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

I strongly discourage you to set on_failure: change. If travis fails this should be a permanent thread to your mailbox until it is fixed properly :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

If travis fails this should be a permanent thread to your mailbox until it is fixed properly

Why? "on_failure: always" -> send mail on every error (default), "on_failure: change" -> send mail only for the first time and after it was resolved(?)

@mathias-luedtke
Copy link
Contributor

My fix is working, but prevents the plugins from beeing used in devel space. So we might consider to remove cob_kinematics (#72)

@fmessmer
Copy link
Member

So we might consider to remove cob_kinematics (#72)

I vote for removing cob_kinematics for the following reasons: #72 (comment)

@fmessmer fmessmer mentioned this pull request Feb 18, 2017
@fmessmer
Copy link
Member

fmessmer commented Mar 5, 2017

replaced by #102

@fmessmer fmessmer closed this Mar 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants