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

Change docker tag to match version, not hardcoded #97

Merged
merged 5 commits into from
May 15, 2024

Conversation

marrts
Copy link
Contributor

@marrts marrts commented Apr 5, 2024

The current {DISTRO}-master dockers are pointing to tesseract_planning {OS}-0.21. This means they don't have the latest changes in tesseract or tesseract_planning. This change makes tesseract_ros2 consistent with tesseract_planning as seen here: https://github.com/tesseract-robotics/tesseract_planning/blob/f22b04dcbe14afe425c63d8bfbb4d14ee4255bcf/.github/workflows/docker.yml#L60

@marrts
Copy link
Contributor Author

marrts commented Apr 5, 2024

Working through some other issues at the moment to get this ready. tesseract-robotics/tesseract_qt#103

TAG=${{ matrix.os }}-0.21
TAG=${{ steps.meta.outputs.version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

steps.meta.outputs.version is version information that is specific to this repository, not the parent image for which TAG is being specified. I think this needs to remain hard-coded to the specific version of the parent image that you want to pull.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't version tags supposed to track across all repos of tesseract?

Copy link
Contributor

@marip8 marip8 Apr 5, 2024

Choose a reason for hiding this comment

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

Yes for convenience they are all currently the same, but they don't technically have to be so this could break down in the future (and is already breaking down because the branch names are not all the same for all tesseract repos)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't version tags supposed to track across all repos of tesseract?

This is not accurate because I see at least TrajOpt follows a different version number and that is what tesseract_planning builds off of.

So would you be okay with something like:

if PR -> reference master/main
if new main/master branch -> reference master/main
if tag -> reference {hard coded tag}

I feel like that most closely aligns with how tesseract PRs have been managed with regards to unstable/stable versions of CI.

@Levi-Armstrong, any input on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think they should have the same major and minor version across all tesseract repositories and trajopt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tesseract_qt was updated to use master instead of main and trajopt is set to match tesseract tags now per: tesseract-robotics/trajopt#397. For these reasons we should be able to proceed with this change.

@marrts marrts merged commit 19707ba into tesseract-robotics:master May 15, 2024
2 of 3 checks passed
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.

3 participants