-
Notifications
You must be signed in to change notification settings - Fork 97
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
Add in artful and bionic to the short os names. #493
Conversation
Signed-off-by: Chris Lalancette <[email protected]>
ros_buildfarm/common.py
Outdated
'trusty': 'T', | ||
'utopic': 'U', | ||
'vivid': 'V', | ||
'wily': 'W', | ||
'xenial': 'X', | ||
'yakkety': 'Y', | ||
'zesty': 'Z', | ||
'artful', : 'A', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax error of extra comma on this and the next two lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, good call. Fixed in a85cc40
ros_buildfarm/common.py
Outdated
'artful', : 'A', | ||
'bionic', : 'B', | ||
'wheezy', : 'W', | ||
'jessie': 'J', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to keep things in alphabetical order. It's not enforced here, but is what we do elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have overridingly strong preferences but I do prefer chronological rather than alphabetical sorting for distro codenames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code currently uses an alphabetical order. When adding new distros that should stay that way.
Reordering could be done in a separate PR (which I agree with @tfoote is not desired imo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I really don't care all that much, I will point out that other places in ros-infrastructure do use Ubuntu-release-order, Debian-release-order: https://github.com/ros-infrastructure/buildfarm_deployment_config/blob/master/hiera/hieradata/buildfarm_role/repo.yaml#L100, https://github.com/ros-infrastructure/rospkg/blob/master/stdeb.cfg#L7 (and the rest of the packages with stdeb.cfg).
That being said, I will change this back to the original order to get this moving.
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
While we are here, change the order of the listing so it is Ubuntu chronological, followed by Debian chronological. This matches what we do in other repositories (like https://github.com/ros-infrastructure/buildfarm_deployment_config/blob/master/hiera/hieradata/buildfarm_role/repo.yaml#L100)
Signed-off-by: Chris Lalancette [email protected]