TSC21 commented 3 years ago

The functionality to set the defaults.yaml file to use wasn't functioning as it was marked, and one needed to add /ros_ws prefix to the path so we were actually able to get and use the file. In the other hand, this was limiting us to the workspace files we could pass as colcon defaults.

This PR basically now allow us to pass any file to be used as default.yaml. The default behavior of setting defaults.yaml, even if it was empty, was kept. Also, I removed the enforcement of the event handlers, and the verbs can now be configured through the colcon defaults.yaml file that is passed. I added an example of how this can be done to the README.

@emersonknapp looking forward for your review and feedback. This works quite nicely now on my end and I am able to set any config file without being restricted to the files in my workspace. And the fact that we don't enforce the event handlers is actually good because then I can use any that I want.

codecov[bot] commented 3 years ago

Codecov Report

Merging #333 (1839874) into master (7b84a81) will decrease coverage by 0.24%. The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master     #333      +/-   ##
==========================================
- Coverage   93.23%   92.99%   -0.25%     
==========================================
  Files          11       11              
  Lines         414      414              
==========================================
- Hits          386      385       -1     
- Misses         28       29       +1

Flag	Coverage Δ
unittests	`92.99% <75.00%> (-0.25%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
ros_cross_compile/dependencies.py	`100.00% <ø> (ø)`
ros_cross_compile/docker_client.py	`97.67% <ø> (-0.16%)`	:arrow_down:
ros_cross_compile/sysroot_creator.py	`98.43% <66.66%> (-1.57%)`	:arrow_down:
ros_cross_compile/ros_cross_compile.py	`85.36% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7b84a81...1839874. Read the comment docs.

TSC21 commented 3 years ago

@ros-tooling/reviewers can someone help review this? Thank you!

TSC21 commented 3 years ago

@emersonknapp would it probably make sense to issue a 0.9.0?

BTW I am close to get the cross-compilation (no qemu builds anymore) to work. I would say I will have it ready in the following week for your review. Then you might finally release a 1.0.0 when that's in :)

emersonknapp commented 3 years ago

would it probably make sense to issue a 0.9.0?

Sure, I can cut a minor release to include latest updates.

BTW I am close to get the cross-compilation (no qemu builds anymore) to work. I would say I will have it ready in the following week for your review. Then you might finally release a 1.0.0 when that's in :)

Wow, sounds good. I look forward to reviewing it. Hopefully it didn't require a full rewrite :)

TSC21 commented 3 years ago

would it probably make sense to issue a 0.9.0?

Sure, I can cut a minor release to include latest updates.

Perfect! Thanks!

BTW I am close to get the cross-compilation (no qemu builds anymore) to work. I would say I will have it ready in the following week for your review. Then you might finally release a 1.0.0 when that's in :)

Wow, sounds good. I look forward to reviewing it. Hopefully it didn't require a full rewrite :)

Not really. There are just some details regarding what has to be put on the sysroot and what has to be put on the build container. For example, if you have dependencies out of the ROS ones, like some build time Python dependency installed with pip or a Java code generator (also build dependency), then we need them on the build image instead of the sysroot. I am planning to add a separate custom_script that the user can use to feed dependencies and others to the build image.

TSC21 commented 3 years ago

@emersonknapp can we get a 0.9.0 meanwhile? Thanks in advance!

emersonknapp commented 3 years ago

done - sorry for the delay https://pypi.org/project/ros-cross-compile/0.9.0/

TSC21 commented 3 years ago

done - sorry for the delay https://pypi.org/project/ros-cross-compile/0.9.0/

No problem. Thank you!

christophebedard commented 2 years ago

BTW I am close to get the cross-compilation (no qemu builds anymore) to work. I would say I will have it ready in the following week for your review. Then you might finally release a 1.0.0 when that's in :)

@TSC21 just wondering, is there any update on this? I think some people in the real-time working group might be interested :smile:

TSC21 commented 2 years ago

BTW I am close to get the cross-compilation (no qemu builds anymore) to work. I would say I will have it ready in the following week for your review. Then you might finally release a 1.0.0 when that's in :)

@TSC21 just wondering, is there any update on this? I think some people in the real-time working group might be interested 😄

No no updates yet. Currently on holidays, and then I have to wrap some other stuff meanwhile. I am working on this as I have time.

ros-tooling / cross_compile

Update colcon defaults usage #333

Codecov Report