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

Puma #2895

Merged
merged 15 commits into from
Oct 24, 2018
Merged

Puma #2895

merged 15 commits into from
Oct 24, 2018

Conversation

LeSim
Copy link
Member

@LeSim LeSim commented Oct 23, 2018

No description provided.

@@ -171,7 +171,7 @@ group :development, :test do
gem 'rspec-rails'

# Deploy
gem 'mina', ref: '343a7', git: 'https://github.com/mina-deploy/mina.git'
gem 'mina', git: 'https://github.com/mina-deploy/mina.git'
Copy link
Contributor

Choose a reason for hiding this comment

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

Y'a une bonne raison pour laquelle on tape encore sur la master et pas sur la dernière release ?

Copy link
Member Author

Choose a reason for hiding this comment

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

je sais pas, faudra voir avec mat

Copy link
Contributor

Choose a reason for hiding this comment

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

A priori la version choisie par @mmagn a quelques commit de plus par rapport à la dernière release, je suppose qu’il y a quelque chose d’utile dedans ?

mina-deploy/mina@v1.2.3...0dd5fdb


# Manually create these paths in shared/ (eg: shared/config/database.yml) in your server.
# They will be linked in the 'deploy:link_shared_paths' step.
set :shared_paths, [
'.env',
set :shared_dirs, [
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi on a encore ces trucs ? C'est pas casse-gueule ?

Copy link
Member Author

@LeSim LeSim Oct 23, 2018

Choose a reason for hiding this comment

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

nop, sur ces dossiers la, tu veux une continuité entre les différentes versions.

config/deploy.rb Outdated
end

namespace :service do
desc "Manage services."
Copy link
Contributor

Choose a reason for hiding this comment

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

Me semble pas que ça soit la bonne description de la tache

Copy link
Member Author

Choose a reason for hiding this comment

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

pas faux, je vais faire sauter toutes les descriptions.

end
end

task :setup do
Copy link
Contributor

Choose a reason for hiding this comment

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

Mettre cette tache avant le deploy ?

Copy link
Member Author

Choose a reason for hiding this comment

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

j'ai l'impression que c'est fait dans les commits suivants

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, fait dans #2901

lib/tasks/deploy.rake Outdated Show resolved Hide resolved
config/deploy.rb Outdated Show resolved Hide resolved
config/deploy.rb Outdated Show resolved Hide resolved
config/deploy.rb Outdated Show resolved Hide resolved
config/deploy.rb Show resolved Hide resolved
to :launch do
queue "/etc/init.d/#{user} upgrade "
queue! %[sudo service delayed_job_#{user!} start]
invoke :'rails:assets_precompile'
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi on garde pas le :force comme avant ?

Copy link
Member Author

Choose a reason for hiding this comment

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

il est passé en parametre de mina par la tache rake

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes yes, je m'interroge sur la raison de ce changement :)

@fredZen fredZen self-requested a review October 23, 2018 16:08
Copy link
Contributor

@fredZen fredZen left a comment

Choose a reason for hiding this comment

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

J’approuve dans le sens où il n’y a pas d’énormité qui me saute aux yeux, et que j’ai l’impression qu’il me faudrait des jours de boulot pour comprendre pourquoi c’est fait comme c’est fait, mais j’avoue c’est surtout un vote de confiance à @LeSim et @mmagn .

@LeSim LeSim merged commit 5b8c640 into dev Oct 24, 2018
@gregoirenovel gregoirenovel deleted the puma branch October 24, 2018 11:24
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