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.

Sign up for GitHub

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

Jump to bottom

[21134] Normalize upload artifact name #103

Merged
merged 2 commits into from
Jun 11, 2024
Merged

[21134] Normalize upload artifact name #103

merged 2 commits into from
Jun 11, 2024

Conversation

JesusPoderoso
Copy link
Contributor

@JesusPoderoso JesusPoderoso commented Jun 10, 2024 •
edited by EduPonz
Loading

Description

This PR adds an intermediate step to detect a sequence of forbidden chars in the artifact name, and replace them (if found) with an underscore (_).
The upload artifact uses the modified name even if no changes were done.

Contributor Checklist

  • Commit messages follow the company guidelines.
  • Changes are API compatible.
  • New features have been added to the versions.md and README.md files (if applicable).

Reviewer Checklist

  • The title and description correctly express the PR's purpose.
  • The Contributor checklist is correctly filled.

@JesusPoderoso JesusPoderoso force-pushed the hotfix/test_report_artifact_name branch 8 times, most recently from fec0102 to 3c04130 Compare June 10, 2024 14:22
@JesusPoderoso
Refs #21134: Normalize matrix name before uploading artifact
1bffb2f 
Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@JesusPoderoso JesusPoderoso force-pushed the hotfix/test_report_artifact_name branch from 3c04130 to 1bffb2f Compare June 10, 2024 14:42
@JesusPoderoso JesusPoderoso changed the title [21134] Normalize matrix name before uploading artifact [21134] Normalize upload artifact name Jun 10, 2024
@JesusPoderoso JesusPoderoso mentioned this pull request Jun 10, 2024
Merged
4 tasks
@JesusPoderoso
Copy link
Contributor Author

JesusPoderoso commented Jun 11, 2024 •
edited
Loading

Results of these changes can be checked in this colcon test step

EduPonz
EduPonz suggested changes Jun 11, 2024
View reviewed changes
Copy link

@EduPonz EduPonz left a comment

Choose a reason for hiding this comment

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

The same change should be applied to the download_artifact action so that the user can set the same name in both and make it work.

EduPonz
EduPonz suggested changes Jun 11, 2024
View reviewed changes
versions.md Show resolved Hide resolved
@JesusPoderoso
Refs #21134: Normalize name also when downloading the artifact
ed55fea 
Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@JesusPoderoso JesusPoderoso force-pushed the hotfix/test_report_artifact_name branch from d2a9d17 to ed55fea Compare June 11, 2024 08:02
@JesusPoderoso JesusPoderoso merged commit bd4f914 into main Jun 11, 2024
1 check passed
@JesusPoderoso JesusPoderoso deleted the hotfix/test_report_artifact_name branch June 11, 2024 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JesusPoderoso @EduPonz