This repository has been archived by the owner. It is now read-only.

Add Docker-specific rake tasks #857

Merged
merged 6 commits into from May 14, 2018

Conversation

Projects
None yet
4 participants
@taquitos
Member

taquitos commented May 14, 2018

  • Prevent conflicts with non-Docker setups
  • See #848 for an example
Add Docker-specific rake tasks
- Prevent conflicts with non-Docker setups
@snatchev

snatchev approved these changes May 14, 2018 edited

Thoughts on wrapping the docker tasks in a namespace :docker?

Otherwise, LGTM

@taquitos

This comment has been minimized.

Member

taquitos commented May 14, 2018

Oh, I didn't realize that was a thing. I'm new to Rake. I'll do some reading and see if it makes sense to use.

Add :docker namespace
update non-docker task to be more rubyesque
@KrauseFx

This comment has been minimized.

Member

KrauseFx commented May 14, 2018

While we talk about this, how much of the code do we want to have in a Rakefile, or should those be separate Ruby/shell files in a different structure?

From what I see, Rake has 2 powerful main features we'd make use of

  1. task :name to easily define tasks in combination with namespaces
  2. Lots of helpers, especially around file system related tasks. I don't think we make use of them yet for fastlane.ci

Either way is fine, I know Rakefile can be structured really well, as you can extend it and provide your own helper methods, to keep the main Rakefile clean 👍

Rakefile Outdated
desc "Bootstrap for running in a Docker container"
task :dev_bootstrap do
sh("bundle install")
sh("brew install node") unless sh("npm -v")

This comment has been minimized.

@KrauseFx

KrauseFx May 14, 2018

Member

This is great

Rakefile Outdated
namespace :docker do
desc "Run in a Docker container in production mode"
task :prod do
sh "bundle exec rackup --host 0.0.0.0 -p 8080 --env production"

This comment has been minimized.

@KrauseFx

KrauseFx May 14, 2018

Member

Maybe I'm just confused here, but didn't you want the 0.0.0.0 to not be in docker?

This comment has been minimized.

@taquitos

taquitos May 14, 2018

Member

Docker needs 0.0.0.0 for the host, while VMs shouldn't should also have it.
-- updated

This comment has been minimized.

@taquitos

taquitos May 14, 2018

Member

You aren't confused, I was 🙃

Rakefile Outdated
desc "Bootstrap for running in a Docker container"
task :dev_bootstrap do
sh("bundle install")
sh("brew install node") unless sh("npm -v")

This comment has been minimized.

@thii

thii May 14, 2018

Contributor

I guess we can't use brew in Linux.

This comment has been minimized.

@KrauseFx

KrauseFx May 14, 2018

Member

nice catch, we could move it out into its own method then to make those checks and use different installation methods

@taquitos

This comment has been minimized.

Member

taquitos commented May 14, 2018

I agree, we should make sure the rakefile is clean, and organized. I also think we should limit the amount of stuff that goes in there.

Rakefile Outdated
@@ -3,20 +3,38 @@ task :dev do
end
task :prod do
sh "bundle exec rackup -p 8080 --env production"
sh "bundle exec rackup -o 0.0.0.0 -p 8080 --env production"

This comment has been minimized.

@thii

thii May 14, 2018

Contributor

Oh, and since we're using long-form flags in other places, let's use --host and --port instead of -o and -p 👍

This comment has been minimized.

@taquitos

taquitos May 14, 2018

Member

Great idea 💯

@taquitos taquitos merged commit be0f2ba into master May 14, 2018

3 checks passed

cla/google All necessary CLAs are signed
fastlane.ci: All rspecs pass All green
Details
fastlane.ci: All rubocops pass All green
Details

@taquitos taquitos deleted the docker_specific_rake branch May 14, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.