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

Merge template files into a single file #487

Merged
merged 20 commits into from
Sep 13, 2018
Merged

Merge template files into a single file #487

merged 20 commits into from
Sep 13, 2018

Conversation

lox
Copy link
Contributor

@lox lox commented Sep 12, 2018

Currently we use some javascript to compile various templates down into a single file. This adds more complexity than it's worth.

This rolls things back into a single template and uses some shell magic to merge the mappings.yml file into the main template.

The major implication of this is that we drop support for our .json urls, for instance previously https://s3.amazonaws.com/buildkite-aws-stack/aws-stack.json and https://s3.amazonaws.com/buildkite-aws-stack/aws-stack.yml were the same thing.

Thoughts on this @toolmantim?

@lox lox added the wip label Sep 12, 2018
@toolmantim
Copy link
Contributor

toolmantim commented Sep 12, 2018

Would people have anything that relies on the JSON URL that couldn’t be switched to YML, such as any Terraform code etc? Probably not. Once we push this it’d start returning a 404 and then they’d go to the releases or readme to see what’s up perhaps?

@toolmantim
Copy link
Contributor

Sounds like a good change tho, now that AWS has native YML. Less transforms the better.

@lox
Copy link
Contributor Author

lox commented Sep 12, 2018

There isn't anything that wouldn't support YAML, I don't think. The Terraform stuff just delegates to Cloudformation which accepts either.

The gnarly thing with removing the aws-stack.json is that if you visited an older version of the README the links would 404. Another option would be that we could leave them in place and move to a new bucket? E.g. https://s3.amazonaws.com/buildkite-aws-stack-v4/latest/aws-stack.yml

I think I'd probably lean in that direction.

@toolmantim
Copy link
Contributor

Old URLs in readmes wont last forever, so I don’t reckon it’s a biggie. Especially if the JSON files are attached to the release in GitHub.

It might confuse people in the future if they accidentally find the URL from somewhere and use it thinking they’d get the latest version 🤷🏼‍♂️

Or keep it around for a while? 🤷🏼‍♂️

@lox
Copy link
Contributor Author

lox commented Sep 12, 2018

Several teams rely on auto-updaters @toolmantim that update their stacks periodically. Removing https://s3.amazonaws.com/buildkite-aws-stack/latest/aws-stack.json might entirely break those updaters, vs it requiring manual intervention to change the bucket (as ours will).

I'm torn on which is better 🤔

@lox
Copy link
Contributor Author

lox commented Sep 12, 2018

Old URLs in readmes wont last forever, so I don’t reckon it’s a biggie. Especially if the JSON files are attached to the release in GitHub.

That isn't a thing presently, is it?

@lox
Copy link
Contributor Author

lox commented Sep 12, 2018

Oh, there are versioned urls in the releases, which will still work.

@lox
Copy link
Contributor Author

lox commented Sep 12, 2018

How about we drop support for latest entirely and use the major version number (e.g https://s3.amazonaws.com/buildkite-aws-stack/v3/aws-stack.json)? That allows folks to auto-update provided there aren't breaking changes.

@lox lox removed the wip label Sep 13, 2018
@lox lox merged commit 8b17edd into master Sep 13, 2018
@lox lox deleted the merge-template-files branch September 13, 2018 01:02
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.

None yet

2 participants