-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Integrate vagrant-triggers plugin functionality into core Vagrant #9713
Integrate vagrant-triggers plugin functionality into core Vagrant #9713
Conversation
Send the defined before and or after triggers in the merge function if triggers exist already
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.
lgtm! some small copy tweaks
page_title: "Vagrant Triggers Configuration" | ||
sidebar_current: "triggers-configuration" | ||
description: |- | ||
Documentation of various configuration options for Vagrnat Triggers |
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.
s/Vagrnat/Vagrant
# Configuration | ||
|
||
Vagrant Triggers has a few various options that can be set which define how the | ||
trigger should behave. |
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.
Vagrant Triggers has a few options to define trigger behavior
|
||
The trigger class takes various options. | ||
|
||
* `action` (symbol, array) - Expected to be a single symbol value, an array of symbols, or a _splat_ of symbols. The first argument that comes after either __before__ or __after__ when defining a new trigger. Can be any valid Vagrant command. It also accepts a special value `:all` which will make the trigger fire for every action. An action can be ignored with the `ignore` setting if desired. |
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.
maybe link to the Vagrant commands here? https://www.vagrantup.com/docs/cli/
_after_ Vagrant commands. | ||
|
||
Each trigger is expected to be given a command key for when it should be fired | ||
during the Vagrant command life cycle. These could be defined as a single key or |
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.
lifecycle
end | ||
``` | ||
|
||
Global and machine scopped triggers will execute in the order that they are |
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.
machine-scoped
@briancain Such an awesome job! 🤘 Feel so honored. |
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.
🎆
lib/vagrant/plugin/v2/trigger.rb
Outdated
# | ||
# @param [Object] env Vagrant environment | ||
# @param [Object] config Trigger configuration | ||
# @param [Object] machine Active Machine |
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.
These should be updated with actual types expected
lib/vagrant/plugin/v2/trigger.rb
Outdated
triggers.each do |trigger| | ||
@logger.debug("Running trigger #{trigger.id}...") | ||
|
||
if !trigger.name.nil? |
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.
if trigger.name
# but can be set as a single String or Symbol | ||
# | ||
# Guests are stored internally as strings | ||
if !@only_on.nil? |
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.
if @only_on
end | ||
|
||
# Commands must be stored internally as symbols | ||
if [email protected]? |
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.
if @ignore
errors.concat errorz["shell provisioner"] if !errorz.empty? | ||
end | ||
|
||
if [email protected]? && [email protected]_a?(String) |
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.
if @name &&
errors << I18n.t("vagrant.config.triggers.name_bad_type", cmd: @command) | ||
end | ||
|
||
if [email protected]? && [email protected]_a?(String) |
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.
if @info &&
errors << I18n.t("vagrant.config.triggers.info_bad_type", cmd: @command) | ||
end | ||
|
||
if [email protected]? && [email protected]_a?(String) |
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.
if @warn &&
lib/vagrant/plugin/v2/trigger.rb
Outdated
index = triggers.index(trigger) unless match == true | ||
end | ||
|
||
if !index.nil? |
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.
if index
|
||
The trigger class takes various options. | ||
|
||
* `action` (symbol, array) - Expected to be a single symbol value, an array of symbols, or a _splat_ of symbols. The first argument that comes after either __before__ or __after__ when defining a new trigger. Can be any valid Vagrant command. It also accepts a special value `:all` which will make the trigger fire for every action. An action can be ignored with the `ignore` setting if desired. A list of valid core commands can be found [here](/docs/cli). |
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.
It should probably be called out here explicitly which commands the triggers will apply to at this point since they will be limited to provider dependent commands.
7148c28
to
afc074f
Compare
@briancain Are there any plans to integrate triggers as provisioners feature? As in emyl/vagrant-triggers#21 (comment) |
@zloyleopold - One of the next things I plan on working on with triggers is the ability to define triggers around various events like the provision step rather than just commands, so yep! Soon. I believe the issue tracking that is here: #8500 |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This pull request integrates the community plugin vagrant-triggers functionality as a core feature of Vagrant. It is a complete rewrite of the plugin.
Resolves #3849
Related issue: emyl/vagrant-triggers#85 /cc @emyl
Related pull request: hashicorp/vagrant-spec#28
Related demo environment: briancain/vagrant-triggers-demo