Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

azure: Remove Scale Sets implementation #1605

Merged
merged 1 commit into from
Aug 11, 2017

Conversation

justaugustus
Copy link
Contributor

@justaugustus justaugustus commented Aug 7, 2017

Not sure what everyone's thoughts are, but we're not using any of the Azure Scale Set code.
This PR removes it.

cc: @alexsomesan @s-urbaniak @mxinden @metral

@alexsomesan
Copy link
Contributor

There were some requests for keeping Scale Sets support, back when we switched to Availability Sets.
However, the current SS code is way unmaintained and I think I doesn’t even build anymore. I suspect it would take more effort to re-align it to the current reference than to actually delete it and back-port the AS version to SS.

So I guess we can get rid of it for now and plan to back-port from AS.

alexsomesan
alexsomesan previously approved these changes Aug 9, 2017
Copy link
Contributor

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

Let drop it for now.

@alexsomesan alexsomesan dismissed their stale review August 9, 2017 13:42

Forgot one thing

@alexsomesan
Copy link
Contributor

Can we keep the -as suffixes?
Like I mentioned, there were requests for the SS implementation so we need to bring that back at some point.

Copy link
Contributor

@metral metral left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@justaugustus justaugustus force-pushed the remove-scalesets branch 2 times, most recently from 20541ba to 1e0a1f8 Compare August 9, 2017 14:27
@justaugustus
Copy link
Contributor Author

@alexsomesan Fixed. Thanks for the review!

@justaugustus
Copy link
Contributor Author

@alexsomesan Rebased to fix structure-check failure on master.
Should be good to go now.

Copy link
Contributor

@alexsomesan alexsomesan left a comment

Choose a reason for hiding this comment

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

LGTM

@alexsomesan
Copy link
Contributor

Setting do-not-test since this code is not exercised from anywhere.

@alexsomesan alexsomesan merged commit 919df90 into coreos:master Aug 11, 2017
squat pushed a commit to squat/tectonic-installer that referenced this pull request Sep 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants