Add tasks to run the node tagging job. #124
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| var self = this; | ||
|
|
||
| waterline.tags.find({}).then(function (tags) { | ||
| var catalogTypes = _(tags) |
Contributor
There was a problem hiding this comment.
I just discovered this usage pattern of lodash a couple weeks ago, glad you are using it.
Contributor
Author
There was a problem hiding this comment.
Heavily leveraged: https://github.com/RackHD/on-tasks/blob/master/lib/jobs/generate-sku.js#L59
Contributor
There was a problem hiding this comment.
Ah, well, hat tip to @halfspiral then :)
Contributor
There was a problem hiding this comment.
I like this chain style, seems very cool :)
Contributor
|
+0, a couple of minor tests that I think might be useful in the long run, but if you spend more than 10 minutes on them I'll feel bad :) |
zyoung51
force-pushed
the
feature/node-tagging
branch
2 times, most recently
from
4b38b72 to
6d5250a
Compare
| friendlyName: 'Generate Tag', | ||
| injectableName: 'Task.Base.Catalog.GenerateTag', | ||
| runJob: 'Job.Catalog.GenerateTag', | ||
| requiredOptions: [], |
Member
There was a problem hiding this comment.
should we assert nodeId as a required option here?
Contributor
|
lib/jobs/generate-tag.js
Outdated
| @@ -0,0 +1,119 @@ | |||
| // Copyright 2015, EMC, Inc. | |||
Contributor
There was a problem hiding this comment.
Still the copyright problem
Contributor
|
👍 after fix the copyright and the clarify the injectableName naming. |
zyoung51
force-pushed
the
feature/node-tagging
branch
from
6d5250a to
6b50781
Compare
Contributor
|
zyoung51
force-pushed
the
feature/node-tagging
branch
from
6b50781 to
2a38ced
Compare
spec/lib/jobs/generate-tag-spec.js
Outdated
| return job.run() | ||
| .then(function() { | ||
| expect(waterline.nodes.addTags).to.have.been.calledOnce; | ||
| expect(waterline.nodes.addTags).to.have.been.calledWith('bc7dab7e8fb7d6abf8e7d6ab', ['Test Tag']); |
Contributor
|
👍 |
Contributor
|
👍 after fix the newly appeared "Line is too long" warning. |
zyoung51
force-pushed
the
feature/node-tagging
branch
from
2a38ced to
21e00ad
Compare
Contributor
|
+1 |
jlongever
added a commit
that referenced
this pull request
Feb 21, 2016
Add tasks to run the node tagging job.
kellylu2sym
pushed a commit
to kellylu2sym/on-tasks
that referenced
this pull request
Aug 8, 2017
Brocade VDX switch integration as part of switch discovery
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@RackHD/corecommitters @uppalk1 @keedya @tannoa2
Depends on RackHD/on-core#88