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

🏁 Citadel EOL #1228

Merged
merged 4 commits into from
Jan 7, 2025
Merged

🏁 Citadel EOL #1228

merged 4 commits into from
Jan 7, 2025

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Dec 17, 2024

Part of #1223

DO NOT MERGE UNTIL Jan 1, 2025

Part of Citadel reaching EOL: #1223

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from j-rivero as a code owner December 17, 2024 22:32
@azeey azeey mentioned this pull request Dec 17, 2024
21 tasks
Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

@azeey
Copy link
Contributor Author

azeey commented Jan 7, 2025

Remove this example line? https://github.com/gazebo-tooling/release-tools/blob/master/bloom/ros_gz/README.md?plain=1#L18

I think this can be left as is, since it's just an example of officially supported ROS/Gazebo pairings. Even though Citadel is EOL, the fact that it was paired with ROS 2 Foxy has not changed.

Add the Foxy - Citadel combination to the List of EOL releases section? https://github.com/gazebo-tooling/release-tools/blob/master/bloom/ros_gz/README.md?plain=1#L37

This list is for unofficial Gazebo/ROS pairings, e.g. Harmonic/Humble while the official one is Fortress/Humble. Since Citadel never had an unofficial pairing, I don't think there's anything to do here.

Remove the citadel block from _dashboard_lib.sh? https://github.com/gazebo-tooling/release-tools/blob/master/terminal-dashboard/_dashboard_lib.sh#L9-L26
Remove the citadel block from table.bash? https://github.com/gazebo-tooling/release-tools/blob/master/terminal-dashboard/table.bash

Is there a reason to do so? Even if it's EOL, I think it's nice to have the ability to generate the citadel dashboard. The table command takes the distro as an argument, so users are not forced to generate Citadel's table if they don't want to.

@j-rivero
Copy link
Contributor

j-rivero commented Jan 7, 2025

Remove this example line? https://github.com/gazebo-tooling/release-tools/blob/master/bloom/ros_gz/README.md?plain=1#L18

I think this can be left as is, since it's just an example of officially supported ROS/Gazebo pairings. Even though Citadel is EOL, the fact that it was paired with ROS 2 Foxy has not changed.

Not having a strong position here but I usually clean up most references to EOL distributions just to reduce the noise for the people reading the options. I'm fine with it to stay.

Add the Foxy - Citadel combination to the List of EOL releases section? https://github.com/gazebo-tooling/release-tools/blob/master/bloom/ros_gz/README.md?plain=1#L37
This list is for unofficial Gazebo/ROS pairings, e.g. Harmonic/Humble while the official one is Fortress/Humble. Since Citadel never had an unofficial pairing, I don't think there's anything to do here.

👍

Remove the citadel block from _dashboard_lib.sh? https://github.com/gazebo-tooling/release-tools/blob/master/terminal-dashboard/_dashboard_lib.sh#L9-L26

Remove the citadel block from table.bash? https://github.com/gazebo-tooling/release-tools/blob/master/terminal-dashboard/table.bash
Is there a reason to do so? Even if it's EOL, I think it's nice to have the ability to generate the citadel dashboard. The table command takes the distro as an argument, so users are not forced to generate Citadel's table if they don't want to.

Same than in first point, not an strong argument but we probably remove previous versions from the table to reduce the number of lines of code that the dashboard is hosting. I'm also fine with it to stay for some months just in case someone needs it a some point.

azeey added 2 commits January 7, 2025 16:23
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey merged commit 0ee823b into master Jan 7, 2025
2 checks passed
@azeey azeey deleted the remove_citadel_ci branch January 7, 2025 22:28
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