search

acquia / moonshot

Moonshot: Because releasing services shouldn't be a moonshot!
Apache License 2.0
53 stars 49 forks source link

CPD-6052 | Minifying template by default. #272

Closed pcostaacquia closed 3 years ago

pcostaacquia commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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. ;-)

https://github.com/acquia/moonshot/pull/273