Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Added NodeStart event #222

Merged
merged 3 commits into from
May 14, 2020
Merged

Added NodeStart event #222

merged 3 commits into from
May 14, 2020

Conversation

arendhil
Copy link
Contributor

@arendhil arendhil commented Mar 4, 2020

Added NodeStart event and implemented a working listener with debug on the Space example scene.

desplesda
desplesda requested changes Mar 8, 2020
View reviewed changes
Copy link
Collaborator

@desplesda desplesda 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! Two things:

  1. can this be done in a way that avoids having to use LINQ?
  2. This wasn't specified elsewhere (my bad!), but can you use 4-space soft tabs instead of hard tabs?

arendhil
arendhil commented Mar 12, 2020
View reviewed changes
Copy link
Contributor Author

@arendhil arendhil left a comment

Choose a reason for hiding this comment

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

Added a NodeStart event where is sent the nodeName and the IEnumerable with the set of tags existing in the node that will be run.

@arendhil arendhil requested a review from desplesda April 2, 2020 16:55
@desplesda desplesda self-assigned this May 13, 2020
@desplesda desplesda added this to the v1.2 milestone May 14, 2020
@desplesda desplesda merged commit 43ba483 into YarnSpinnerTool:develop May 14, 2020
@desplesda
Copy link
Collaborator

Thank you very much! I made some changes to make this line up a little better with existing API, and added documentation comments.

@arendhil arendhil deleted the feature/node-start-event branch May 29, 2020 00:17
@arendhil
Copy link
Contributor Author

Thanks, I'll take a look!

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@desplesda desplesda Awaiting requested review from desplesda

Assignees

@desplesda desplesda

Labels
None yet
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.
2 participants
@arendhil @desplesda