Skip to content

Allow external use of write_settings#386

Merged
tobywf merged 1 commit into
aws-cloudformation:masterfrom
stilvoid:allow-write-settings
Jan 31, 2020
Merged

Allow external use of write_settings#386
tobywf merged 1 commit into
aws-cloudformation:masterfrom
stilvoid:allow-write-settings

Conversation

@stilvoid
Copy link
Copy Markdown
Contributor

Issue #, if available: N#A

Description of changes: Allows plugins to call write_settings on the project. We need this in the Go plugin as we're intending to store plugin version information in .rpdk-config. See aws-cloudformation/cloudformation-cli-go-plugin#115

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@stilvoid
Copy link
Copy Markdown
Contributor Author

The test failure reported by Travis is also failing in the master branch and is not introduced by this PR. I'll rebase when it's fixed.

@stilvoid
Copy link
Copy Markdown
Contributor Author

The test failure reported by Travis is also failing in the master branch and is not introduced by this PR. I'll rebase when it's fixed.

Fix in #388

@tobywf
Copy link
Copy Markdown
Contributor

tobywf commented Jan 30, 2020

I can't think of/remember a reason not to do this, but I also can't figure out why we didn't do this in the first place - and there must of been a reason for us to go out of our way to not have language be a project attribute in such a blatant way. Let me try and sync up with John about this.

@stilvoid stilvoid force-pushed the allow-write-settings branch from 3d1ee5c to 316cb75 Compare January 31, 2020 08:54
Copy link
Copy Markdown
Contributor

@johnttompkins johnttompkins 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. I think previously we just didn't have a use for language in the project other than loading the plugin on subsequent commands.

@tobywf
Copy link
Copy Markdown
Contributor

tobywf commented Jan 31, 2020

Yeah, I really can't remember. Oh well, ship it :D

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