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

Removes dashing support. #253

Merged
merged 3 commits into from
May 20, 2022

Conversation

francocipollone
Copy link
Collaborator

Part of #249

agalbachicar
agalbachicar previously approved these changes May 19, 2022
Copy link
Collaborator

@agalbachicar agalbachicar left a comment

Choose a reason for hiding this comment

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

LGTM

@agalbachicar
Copy link
Collaborator

BTW, aren't you removing the docker images for bionic?

@francocipollone
Copy link
Collaborator Author

BTW, aren't you removing the docker images for bionic?

Forgot to comment about it, thanks for asking.
I haven't removed the dockerfiles for bionic but I modified the scripts to use focal by default.

Even though we are removing support to 18, I felt that there is no harm about having the script allowing you to set up a 18.04 container if needed. However, for consistency I think I should remove it and avoid having to maintain this in the future. Wdyt? I guess you're inclined to remove it, right?

@agalbachicar
Copy link
Collaborator

Git tracks everything :) No harm in removing it.
We could document in the docs the last commit with dashing support though.

@francocipollone
Copy link
Collaborator Author

We could document in the docs the last commit with dashing support though.

Interesting to extend it for all the repos not only maliput_infra. Adding in maliput_documentation a note with the hashes of all the repos with official dashing support?

@agalbachicar
Copy link
Collaborator

Right.

@agalbachicar
Copy link
Collaborator

Actually, that's what tags are for :)

@francocipollone
Copy link
Collaborator Author

Actually, that's what tags are for :)

To continue here: #254

@francocipollone
Copy link
Collaborator Author

CI passing. maliput/delphyne#842 Is fixing delphyne test.

Copy link
Collaborator

@agalbachicar agalbachicar left a comment

Choose a reason for hiding this comment

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

LGTM

@francocipollone francocipollone merged commit dd7dfd1 into main May 20, 2022
@francocipollone francocipollone deleted the francocipollone/remove_dashing_support branch May 20, 2022 19:13
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.

2 participants