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

CPD-6052 | Minifying template by default. #272

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

pcostaacquia
Copy link

No description provided.

@pcostaacquia
Copy link
Author

pcostaacquia commented Feb 22, 2021

Manual test

mbpsujo:cloud-database-api pcosta$ MOONSHOT_SSH_USER=pcosta CDB_CI_USER=pcosta    bundle exec launcher launch --api-name=dev-${USER}                --worker-name=dev-${USER} --network-name=dev-${USER}                --new-relic-license-key=4cb9f4bdb64c601162dd7c13539e19f0a0695a40586
Version 2 of the Ruby SDK will enter maintenance mode as of November 20, 2020. To continue receiving service updates and new features, please upgrade to Version 3. More information can be found here: https://aws.amazon.com/blogs/developer/deprecation-schedule-for-aws-sdk-for-ruby-v2/
Checking bundle in /Users/pcosta/git/cloud-database-api/lib/acquia/cloud/data/tools/cli/../../../../../../.././cloud-database-api.
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /Users/pcosta/git/cloud-database-api/lib/acquia/cloud/data/tools/cli/launcher.rb:513)
The Gemfile's dependencies are satisfied
Checking bundle in /Users/pcosta/git/cloud-database-api/lib/acquia/cloud/data/tools/cli/../../../../../../.././cloud-database-worker.
The Gemfile's dependencies are satisfied
Writing launcher logs to /Users/pcosta/git/cloud-database-api/cdb-launcher-launch20210222-72898-4eeyh
Launching: |

Screen Shot 2021-02-22 at 18 38 35

@pdrakeweb
Copy link
Contributor

This looks good to me, but I have two questions:

  1. Do we want this to be applicable to dynamic templates only, or should we do this in a more generic way that also applies to static templates?
  2. What do you think about creating a PR against master first, and then having the PR against the Cloud Data branch be a cherry-pick of the same commit?

@pcostaacquia
Copy link
Author

pcostaacquia commented Feb 23, 2021

This looks good to me, but I have two questions:

  1. Do we want this to be applicable to dynamic templates only, or should we do this in a more generic way that also applies to static templates?
  2. What do you think about creating a PR against master first, and then having the PR against the Cloud Data branch be a cherry-pick of the same commit?

Thank you very much for the feedback !!!

About the first point, hum ... we might be more comprehensive, yes. It's my first moonshot PR, passing through those classes I think the one you are referencing to is json_stack_template.rb, isn't that right ? Well, in this class we have already:

@template_body ||= JSON.parse(File.read(@filename))

So we probably ok here.

While we figure out if this is enough I will re open the first PR I created against master. ;-)

#273

@marciotoshio marciotoshio merged commit 2830360 into acquia:proxy-cli-hook-aws-v2 Mar 4, 2021
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