-
Notifications
You must be signed in to change notification settings - Fork 95
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
Engine events #230
Engine events #230
Conversation
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 good to me in general 👍 only some minor comments added in line, please consider those before merging. Thanks!
*/ | ||
public class EngineEvent { | ||
private static final Pattern PATTERN = | ||
Pattern.compile("^salt/engines/([^/]+)/(.*)$"); |
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 there is no need to break this line (and maybe others below) anymore, max line length for checkstyle was meanwhile increased to 120 characters!
private final JsonElement data; | ||
|
||
/** | ||
* Creates a new BeaconEvent |
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.
This is not a BeaconEvent
anymore, please update to EngineEvent
.
.gitignore
Outdated
@@ -15,3 +15,4 @@ | |||
|
|||
# OSX Metadata | |||
.DS_Store | |||
/bin/ |
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.
Actually when you configure your eclipse project as a "Maven Project", which I would recommend you as it makes building and tracking dependencies easier, there is no need for adding the bin
folder: the output folder will be ./target/classes
per default which is already gitignored.
engine events are somewhat special since they can be fired from either a master or a minion. Add an event class to handle them.
Merged it, thank you @cbosdo! |
Adding engine event class and its test handling both master and minion engine events.