Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Catkin should send out only one notification #358

Merged
merged 4 commits into from
Apr 24, 2016

Conversation

cbandera
Copy link
Contributor

I have noticed, that catkin sends out several notifications after a build instead of sending one summary.

This leads to a poor user experience, as you might miss some information or even get a wrong idea about the success of you build as in this example:
Build a workspace with packages that produce warnings. After the build, you will get two notifications:

  • "Build Suceeded. 5 packages completed with no warnings"
    catkinsuccessnotification
  • "Build Produced Warnings. 1 packages succeeded with warnings"
    catkinwarningnotification

This is especially confusing on system, where notifications don't stack, but really show up one after another, so that at first glance you only see half of the information.

I have therefor merged all information that is produced by the print_compact_summary function into a single call to notify, passing over all information at once.
catkinsummarynotification

@jbohren
Copy link
Contributor

jbohren commented Apr 22, 2016

This definitely looks like an improvement. Now, of course, I want there to be a yellow/orange icon when there are warnings!

@jbohren jbohren added this to the 0.4.x - Beta 2 Patches milestone Apr 22, 2016
@jbohren
Copy link
Contributor

jbohren commented Apr 22, 2016

@cbandera Our pedantic CI rig just needs the following to be fixed, then this should be good to merge after one of us tries it out.

../home/travis/build/catkin/catkin_tools/tests/../catkin_tools/execution/controllers.py:367:81: E231 missing whitespace after ','
/home/travis/build/catkin/catkin_tools/tests/../catkin_tools/execution/controllers.py:376:31: E231 missing whitespace after ','
/home/travis/build/catkin/catkin_tools/tests/../catkin_tools/execution/controllers.py:389:87: E231 missing whitespace after ','

@jbohren
Copy link
Contributor

jbohren commented Apr 23, 2016

c512b03 Awesome!

@wjwwood
Copy link
Member

wjwwood commented Apr 24, 2016

Thanks, this is great!

@wjwwood wjwwood merged commit b9963df into catkin:master Apr 24, 2016
@cbandera cbandera deleted the feature/single_notification branch April 25, 2016 06:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants