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

Add measurement-affecting .with_args to an example #590

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Conversation

fahhem
Copy link
Collaborator

@fahhem fahhem commented Jun 7, 2017

No description provided.

@fahhem fahhem requested a review from kdsudac June 7, 2017 22:34
Copy link
Collaborator

@kdsudac kdsudac left a comment

Choose a reason for hiding this comment

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

Just one nit

"""This tests that we can ping a host.

The plug, pinger, is expected to be replaced at test creation time, so the
placeholder property was used instead of the class directly.
"""
del expected_retcode # Not used in the phase, only used by a measurement
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd probably leave this line out because I think it is likely to confuse people new to htf.

You are really deleting only for linter, not really because it's a good practice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm making it clear that it has to be an input argument but doesn't have to be used in the phase

@fahhem fahhem merged commit aa405d8 into master Jun 13, 2017
@fahhem fahhem deleted the fahhem-patch-8 branch July 21, 2017 02:00
DanLipsitt pushed a commit to ShaperTools/openhtf that referenced this pull request Aug 10, 2017
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.

2 participants