Skip to content

Conversation

@mergify
Copy link

@mergify mergify bot commented Mar 23, 2023

This is an automatic backport of pull request #1908 done by Mergify.
Cherry-pick of 39d3714 has failed:

On branch mergify/bp/humble/pr-1908
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit 39d371453.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   .docker/source/Dockerfile

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   .docker/ci-testing/Dockerfile
	both modified:   .github/workflows/ci.yaml

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

Co-authored-by: Vatan Aksoy Tezer <[email protected]>
(cherry picked from commit 39d3714)

# Conflicts:
#	.docker/ci-testing/Dockerfile
#	.github/workflows/ci.yaml
@mergify mergify bot added the conflicts label Mar 23, 2023
@tylerjw tylerjw requested a review from henningkayser March 23, 2023 20:20
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

@vatanaksoytezer pinging you here just in case you want to take a look. We are trying to make CI green on the humble branch as well (see #2040)

Comment on lines 4 to 12
ARG ROS_DISTRO=rolling
<<<<<<< HEAD
FROM osrf/ros2:testing
=======
FROM moveit/moveit2:${ROS_DISTRO}-ci
>>>>>>> 39d371453 (Temporarily switch to dockerhub for CI jobs (#1908))
LABEL maintainer Robert Haschke [email protected]

ENV TERM xterm
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also incorporate the changes from #2035?

Suggested change
ARG ROS_DISTRO=rolling
<<<<<<< HEAD
FROM osrf/ros2:testing
=======
FROM moveit/moveit2:${ROS_DISTRO}-ci
>>>>>>> 39d371453 (Temporarily switch to dockerhub for CI jobs (#1908))
LABEL maintainer Robert Haschke [email protected]
ENV TERM xterm
FROM osrf/ros2:testing
LABEL maintainer Robert Haschke [email protected]
ENV TERM xterm
# Overwrite the ROS_DISTRO set in osrf/ros2:testing to the distro tied to this Dockerfile (OUR_ROS_DISTRO).
# In case ROS_DISTRO is now different from what was set in osrf/ros2:testing, run `rosdep update` again
# to get any missing dependencies.
# https://docs.docker.com/engine/reference/builder/#using-arg-variables explains why ARG and ENV can't have
# the same name (ROS_DISTRO is an ENV in the osrf/ros2:testing image).
ARG OUR_ROS_DISTRO=rolling
ENV ROS_DISTRO=${OUR_ROS_DISTRO}
RUN rosdep update --rosdistro $ROS_DISTRO

Copy link
Contributor

Choose a reason for hiding this comment

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

If the changes from #2035 are made here as well, the corresponding github workflow build arg should be renamed to OUR_ROS_DISTRO: https://github.com/ros-planning/moveit2/blob/mergify/bp/humble/pr-1908/.github/workflows/docker.yaml#L134

Comment on lines +34 to +42
<<<<<<< HEAD
DOCKER_IMAGE: ghcr.io/ros-planning/moveit2:${{ matrix.env.IMAGE }}
UPSTREAM_WORKSPACE: moveit2.repos $(f="moveit2_$(sed 's/-.*$//' <<< "${{ matrix.env.IMAGE }}").repos"; test -r $f && echo $f)
=======
DOCKER_IMAGE: moveit/moveit2:${{ matrix.env.IMAGE }}
UPSTREAM_WORKSPACE: >
moveit2.repos
$(f="moveit2_$(sed 's/-.*$//' <<< "${{ matrix.env.IMAGE }}").repos"; test -r $f && echo $f)
>>>>>>> 39d371453 (Temporarily switch to dockerhub for CI jobs (#1908))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<<<<<<< HEAD
DOCKER_IMAGE: ghcr.io/ros-planning/moveit2:${{ matrix.env.IMAGE }}
UPSTREAM_WORKSPACE: moveit2.repos $(f="moveit2_$(sed 's/-.*$//' <<< "${{ matrix.env.IMAGE }}").repos"; test -r $f && echo $f)
=======
DOCKER_IMAGE: moveit/moveit2:${{ matrix.env.IMAGE }}
UPSTREAM_WORKSPACE: >
moveit2.repos
$(f="moveit2_$(sed 's/-.*$//' <<< "${{ matrix.env.IMAGE }}").repos"; test -r $f && echo $f)
>>>>>>> 39d371453 (Temporarily switch to dockerhub for CI jobs (#1908))
DOCKER_IMAGE: moveit/moveit2:${{ matrix.env.IMAGE }}
UPSTREAM_WORKSPACE: >
moveit2.repos
$(f="moveit2_$(sed 's/-.*$//' <<< "${{ matrix.env.IMAGE }}").repos"; test -r $f && echo $f)

@vatanaksoytezer
Copy link
Contributor

@adlarkin thank you for the ping. Please feel free to close this and open a new PR with the changes you want. I might not have time to look at this in the next few days. (Sorry extremely busy week)

@adlarkin adlarkin mentioned this pull request Apr 5, 2023
6 tasks
@adlarkin
Copy link
Contributor

adlarkin commented Apr 5, 2023

No worries, sorry for the delay on my end as well - I have just opened a replacement PR and will close this one: see #2081

@adlarkin adlarkin closed this Apr 5, 2023
@mergify mergify bot deleted the mergify/bp/humble/pr-1908 branch April 5, 2023 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants