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

Breakout app engine #144

Conversation

thefirstofthe300
Copy link
Contributor

Builds off of the commits made by Nick Stogner in #134.

Refactors project factory's App Engine configuration to no longer use the deprecated and now removed app_engine_config block from the google_project resource.

morgante
morgante previously approved these changes Feb 22, 2019
Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

Let's drop the replication of the app_engine submodule interface and just require the user to invoke that module directly.

docs/upgrading_to_project_factory_v2.0.md Outdated Show resolved Hide resolved
@thefirstofthe300 thefirstofthe300 force-pushed the breakout-app-engine branch 4 times, most recently from 7ccc0b1 to 145e66b Compare February 22, 2019 18:58
@thefirstofthe300 thefirstofthe300 marked this pull request as ready for review February 22, 2019 20:18
Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

A documentation fix, and some obsolete variables to drop. Please regenerate the documentation after removing the variables.

docs/upgrading_to_project_factory_v2.0.md Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
modules/core_project_factory/variables.tf Outdated Show resolved Hide resolved
aaron-lane
aaron-lane previously approved these changes Feb 22, 2019
@aaron-lane
Copy link
Contributor

This looks good, but let's hold off from merging until the regression in 1.1.0 is fixed.

Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

Some readability suggestions for your consideration.

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/upgrading_to_project_factory_v2.0.md Outdated Show resolved Hide resolved
docs/upgrading_to_project_factory_v2.0.md Outdated Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
modules/app_engine/outputs.tf Outdated Show resolved Hide resolved
aaron-lane
aaron-lane previously approved these changes Feb 27, 2019
@aaron-lane
Copy link
Contributor

Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

The feature_settings variable needs some attention as described below.

Can you regenerate the documentation using the latest version of terraform-docs to ensure that the escaping isn't removed.

modules/app_engine/variables.tf Outdated Show resolved Hide resolved
aaron-lane
aaron-lane previously approved these changes Mar 5, 2019
@aaron-lane aaron-lane merged commit b0c1aea into terraform-google-modules:master Mar 5, 2019
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.

4 participants