Open
Conversation
jbarlow-mcafee
approved these changes
Jul 30, 2018
Contributor
jbarlow-mcafee
left a comment
jbarlow-mcafee
left a comment
There was a problem hiding this comment.
This looks good to me.
As mentioned in opendxl/opendxl-epo-client-python#7 (comment), it might be good for consistency to replace the BaseClientTest.run_sample calls with self.run_sample calls instead.
In the original PR, #5 (review), I suggested that it might be good to add a tests which validates that the results from a search are translated properly into the various properties in the ResultsContext object. What do you think of this idea? I'd be okay with having that done as a separate PR and merging this one as-is if that's what you'd prefer.
Contributor
|
Ah, sorry, this will now need to rebased since I just merged in a commit to fix the pylint-related failures in Travis on the current master branch. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
No description provided.