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

Migrate some amazon code to typescript #9742

Merged

Conversation

christopherthielen
Copy link
Contributor

@christopherthielen christopherthielen commented Oct 5, 2021

The bulk of this change is AwsInstanceTypeService and AwsServerGroupCommandBuilder
Github isn't really tracking all the renames so the diff looks awful. Review if you can/want. I hope I didn't break anything.

@christopherthielen christopherthielen force-pushed the amazon-typescript-migration branch 4 times, most recently from 88a0100 to 835a0c6 Compare October 9, 2021 00:36
@christopherthielen christopherthielen marked this pull request as ready for review October 9, 2021 00:38
@@ -39,7 +39,7 @@ export interface ILicenseConfig {
export interface IMetadataOptions {
httpEndpoint?: 'disabled' | 'enabled';
httpPutResponseHopLimit?: number;
httpsTokens?: 'required' | 'optional';
httpTokens?: 'required' | 'optional';
Copy link
Member

Choose a reason for hiding this comment

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

Not familiar at all with the AWS code base, but purely from an interest standpoint what's the reasoning for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this interface was incorrectly written as httpsTokens but the backend expects httpTokens. The code that exercised this was Javascript, not Typescript so the incorrect interface definition wasn't caught earlier.

Copy link
Member

@mattgogerly mattgogerly 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 to me from a JS standpoint. Only one question.

@christopherthielen christopherthielen force-pushed the amazon-typescript-migration branch 2 times, most recently from 8e13436 to b137ab5 Compare October 26, 2021 18:30
@christopherthielen christopherthielen added ready to rebase Reviewed and ready to rebase and merge self merge labels Oct 26, 2021
@mergify mergify bot merged commit 9a3dbe5 into spinnaker:master Oct 26, 2021
@christopherthielen christopherthielen deleted the amazon-typescript-migration branch October 26, 2021 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to rebase Reviewed and ready to rebase and merge self merge target-release/1.28
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants