search

bazaarvoice / cloudformation-ruby-dsl

Ruby DSL for creating Cloudformation templates
Apache License 2.0
210 stars 76 forks source link

Make options optional again #103

Closed KierranM closed 7 years ago

KierranM commented 7 years ago

e712636 changed the arguments to #raw_template to a single options hash, unfortunately the options argument is not optional in itself. This caused a bunch of our code that extends the dsl and uses #raw_template to break as they were not using in arguments.

That change was also released under a patch version bump which is also unfortunate as it meant even a pessimistic version lock would not have prevented it from breaking. I believe releasing such a change should have been a major bump according guidelines set forth in your release documentation.

The change in this PR makes the argument optional once again and sets the values to what they would have been, had they not been given.

jonaf commented 7 years ago

Hi @KierranM , thanks for the pull request! I think the changes from #87 should have been additions only, and not a breaking change, and so the fact that it broke should be considered a bug. Since it was intended to only add, I should have considered it a minor release, not a patch release, so that was clearly my mistake. Sadly, since I've already released it, I'm not able to do much more than release another patch with your bugfix, in hopes of minimizing the impact of the breaking change.

temujin9 commented 7 years ago

@jonaf The way to correct this, in release paths, is to revert the offending commit as a new commit, release that as yet another patch on the broken minor, and then cherry-pick the offending commit (plus fix?) into a new minor.

That way, pessimistic inclusion (which updates on patches, but nothing else) should work as expected.

temujin9 commented 7 years ago

Okay, got releases 1.2.5 and 1.3.0 done per this discussion. This looks good, and will become 1.3.1. Thanks!