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

Engine events #230

Merged
merged 1 commit into from
Apr 10, 2018
Merged

Engine events #230

merged 1 commit into from
Apr 10, 2018

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Apr 6, 2018

Adding engine event class and its test handling both master and minion engine events.

@renner renner self-requested a review April 10, 2018 08:53
@renner renner added this to the Version 0.14.0 milestone Apr 10, 2018
Copy link
Member

@renner renner left a 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/([^/]+)/(.*)$");
Copy link
Member

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
Copy link
Member

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/
Copy link
Member

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.
@renner renner merged commit 152384e into SUSE:master Apr 10, 2018
@renner
Copy link
Member

renner commented Apr 10, 2018

Merged it, thank you @cbosdo!

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.

None yet

2 participants