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

(contributing) Add guidelines for anchoring and aliasing of pip entries #29037

Merged
merged 4 commits into from
May 6, 2021
Merged

(contributing) Add guidelines for anchoring and aliasing of pip entries #29037

merged 4 commits into from
May 6, 2021

Conversation

MatthijsBurgh
Copy link
Contributor

@MatthijsBurgh MatthijsBurgh commented Apr 8, 2021

As discussed in #28891

Fixes #28891

@MatthijsBurgh MatthijsBurgh mentioned this pull request Apr 8, 2021
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks for the followup. I think that this will work. The name is not completely self explanatory but having this document written out hopefully will help people find the full meaning.

One typo to fixup before merge.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@tfoote tfoote requested a review from a team April 13, 2021 02:28
Co-authored-by: Tully Foote <[email protected]>
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

Could you add clarification to the document what date should be used in the anchor?

CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Scott K Logan <[email protected]>
Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

In general I think we have to be very cautious when using advanced yaml features both because they're not implemented in all parsers (especially in safe modes) and because they aren't clear to casual yaml editors.

It's a bummer that we have to have the anchors before the aliases as it will create more churn when remove the python- key and move the content to the aliased python3- key.

I cannot think of a different way to solve this same problem and ultimately this is an issue of transition which means that we'll eventually be through it.

Well done @MatthijsBurgh on pushing through and finding a solution to a nasty problem.

@MatthijsBurgh
Copy link
Contributor Author

@cottsay @ivanpauno friendly ping ;)

@MatthijsBurgh
Copy link
Contributor Author

@cottsay @ivanpauno ping ;)

@nuclearsandwich
Copy link
Member

@MatthijsBurgh thanks for iterating with our team to propose a solution. I'm glad we came up with something that while a bit rough, is no rougher than the problem space, and has an explicit expiration date.

@nuclearsandwich nuclearsandwich merged commit cde8f75 into ros:master May 6, 2021
@MatthijsBurgh MatthijsBurgh deleted the pip_anchor_alias branch May 6, 2021 19:19
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.

Pip entries
5 participants