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

update amazon platform mapping to support Versions 1, 2, and 2 RCs #264

Merged
merged 2 commits into from
Aug 8, 2018

Conversation

wrightp
Copy link

@wrightp wrightp commented Jul 26, 2018

@wrightp wrightp requested a review from tas50 July 26, 2018 20:27
@wrightp wrightp requested a review from a team as a code owner July 26, 2018 20:27
Signed-off-by: Patrick Wright <patrick@chef.io>
@wrightp
Copy link
Author

wrightp commented Aug 6, 2018

@chef/jex-team @tas50 Please review. Thank you.

Copy link
Contributor

@schisamo schisamo left a comment

Choose a reason for hiding this comment

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

Looks good but @tas50 is the expert on all things Amazon Linux detection.

@wrightp
Copy link
Author

wrightp commented Aug 8, 2018

I've tested this on amazon linux platforms.

@wrightp wrightp merged commit dafd96d into master Aug 8, 2018
@chef-ci chef-ci deleted the wrightp/ami2 branch August 8, 2018 16:10
@tas50
Copy link
Contributor

tas50 commented Aug 8, 2018

I'm really confused by this PR. It goes against the meeting we had. We had agreed to keep the logic from being client side, but this moves the Amazon platform mapping back to the client side where we can't easily update it in the future if something changes on the OS side. Additionally we lose the ability to know how many people are actually requesting packages on Amazon, which is a big question we keep getting asked at a management level. It all looks like RHEL6/7 with this.

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