ipa320 / care-o-bot

http://www.care-o-bot.de
Apache License 2.0
48 stars 41 forks source link

Migration to package format 2 #7

Closed fmessmer closed 8 years ago

fmessmer commented 8 years ago

@ipa-fmw @ipa-nhg @ipa-mdl @ipa-mig @ipa-rmb @ipa-bnm

https://github.com/pulls?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+user%3Aipa320+Migration+package+format+2

Let's use this Issue for general discussion about what's still missing for a successfull migration to package format 2:

Remaining repos to be migrated:

Remaining decisions to be applied wherever it occurs:

Please contribute to the discussion in case something is not yet mentioned here...

mathias-luedtke commented 8 years ago

+1 for not installing headers for"executable-only" packages

mathias-luedtke commented 8 years ago
mgruhler commented 8 years ago

+1 for not installing headers of "executable-only" packages.

mgruhler commented 8 years ago

How should boost dependencies be handled? find_package it when used directly, maybe with COMPONENTS REQUIRED? How should it be in the package.xml?

benmaidel commented 8 years ago

+1 for not installing headers of "executable-only" packages

fmessmer commented 8 years ago

I'd suggest to be consequent and add explicit dependencies for boost! I also already started to clean-up executable-only packages (see cob_command_tools)

fmessmer commented 8 years ago

Also, how about packages where there are libraries? In case the libraries are only used within the same packages, they do not need to be exported, right?

This would mean:

How about libraries, that are used by other packages, e.g. cob_generic_can or cob_canopen_motor?

What about plugins? E.g. cob_omni_drive_controller?

@ipa-mdl could you bring in some light here?

fmessmer commented 8 years ago

I'd postpone reviewing "everything-tests-related" to the new issue #8

fmessmer commented 8 years ago

What are we going to to about tags like this (e.g. from here):

if(CMAKE_COMPILER_IS_GNUCXX)
  add_definitions(-std=gnu++0x)
else()
  add_definitions(-std=c++0x)
endif()
mathias-luedtke commented 8 years ago

You are right with the libraries. Plug-ins have to be built and installed, headers are not needed. And the plugin xml has to be installed, of cause.

C++11 code should be avoided for now if possible. I had a brief look at cob_base_velocity_smoother, I think it does not need c++11.

C++11 is will be okay for executables, but probably not for librares, since the c++11 flags must be passed to dependent packages and I do not know if this works.

mgruhler commented 8 years ago

So we have to decide how to handle the libraries, right? IMO, installing libraries and headers will allow other people to use this functionality. Will also make things easier, if someone from IPA starts working on a node using a library, that is in another package.

So my vote is for installing (generally) installing libraries and headers with the respective CATKIN_DEPEND with the exception of where it will definitely not be used somewhere else (e.g. cob_undercarriage_ctrl due to getting deprecated)

mgruhler commented 8 years ago

So boost should only be addes as dependency, when any boost header is explicitely included. right?

mathias-luedtke commented 8 years ago

Boost should be added at least if certain components are used (system, threads, file_system).

If the libs should be used elsewhere (i.e. are useful for others, too) they should be installed. Each lib needs its headers installed!

fmessmer commented 8 years ago

So most of the packages have been migrated and merged!

Remaining repos to be migrated:

  • cob_extern
  • cob_perception_common
  • cob_perception_data
  • cob_navigation
  • cob_android
  • care-o-bot (this package)
  • ...

@ipa-fmw @ipa-nhg @ipa-mdl @ipa-mig @ipa-rmb @ipa-bnm So, who is volunteering to migrate the packages above?

mgruhler commented 8 years ago

I'll take over cob_navigation. And I can also do cob_extern.

benmaidel commented 8 years ago

i take cob_android

ipa-nhg commented 8 years ago

I take care-o-bot and cob_test_robots

benmaidel commented 8 years ago

and my votes libraries:

plugins:

boost:

fmessmer commented 8 years ago

@ipa-bnm That's the way it has been done now for the packages that are already migrated....

mgruhler commented 8 years ago

ipa320/cob_navigation#50 and ipa320/cob_extern#48 done.

fmessmer commented 8 years ago

Awesome :+1:

fmessmer commented 8 years ago

I'll start triggering new releases...later the day/evening...@ipa-fmw @ipa-nhg FYI

mgruhler commented 8 years ago

I’d like to do this for cob_navigation as well. Can you maybe show me how to do this some time this/next week?

Von: Felix Messmer [mailto:notifications@github.com] Gesendet: Dienstag, 25. August 2015 14:45 An: ipa320/care-o-bot Cc: Gruhler, Matthias Betreff: Re: [care-o-bot] Migration to package format 2 (#7)

I'll start triggering new releases...later the day/evening...

— Reply to this email directly or view it on GitHubhttps://github.com/ipa320/care-o-bot/issues/7#issuecomment-134573026.

fmessmer commented 8 years ago

New Releases triggered for Indigo:

Did I forget something?

fmessmer commented 8 years ago

I decided to go on with the rest of the repositories... ...as releasing can easily be done, we could release in shorter cycles whenever a new fix is introduced within one of the repos!

fmessmer commented 8 years ago

All :green_heart: :wink:

mgruhler commented 8 years ago

:clap:

floweisshardt commented 8 years ago

schunk_robots will not be released