-
Notifications
You must be signed in to change notification settings - Fork 117
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
When using deploy -d
option, do not warn when pushing to development
#372
base: master
Are you sure you want to change the base?
Conversation
@@ -40,7 +40,7 @@ def push | |||
|
|||
# Ask for a confirmation (Warning) if we deploy with the -d option | |||
# since it alters content on the remote engine | |||
if options[:data] | |||
if options[:data] && self.env != 'development' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not thought about that. In that case, I think we should a ruby constant like DEVELOPMENT_ENV_NAMES
or DEVELOPMENT_ENVS
or perhaps a method named is_development_env?
which lists all the possible names used for a non prod env (development, dev, test, staging, ...etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about this already. I think self.env== 'production'
would be better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same problem here. I use production
most of the time but sometimes I also use hosting
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm rather than an environment variable, I propose another key added to the config/deploy.yml
like do_not_ask
or is_development_env
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we merge the current self.env== 'production'
in the meantime until a better solution is commited?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just working on a site now, and ran into this same thing... wanting to pull and then push with -d
to override. What if there was just an additional flag --force
or similar that skipped the message? I think it's valuable to have the guard there normally, but make it obvious when it's going to be overridden.. @did If you agree, I could put this together as a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we know what we're doing and get rid of it altogether? 🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's best practice to have the warning there, tedious as it may be. I think a key in the config.yml file is the best option as, IMO, tying a specific term to a behaviour (e.g. development or production = no warning) doesn't fit with Locomotive CMS' customisable ethos. I'd be in support a --force flag though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the --force
solution has been approved by everybody here 👍
@boie0025 go go! 🙏
Turns out the yes/no question for
deploy -d
option is a little bit monotonous in development environment. Lets skip if for development env.