-
Notifications
You must be signed in to change notification settings - Fork 74
Conversation
sachs7
commented
Feb 21, 2017
- import library as per PEP8.
- Update to test_group.py to use UUID for unique name
Use UUID for group_name
- import library as per PEP8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Carrying over r+ from earlier pull (290), and based on my own review + ad-hoc run:
https://gist.github.com/stephendonner/d9453de236f8104630df60e979479d04
Thanks for the PR, @sachs7 - really great to have you contributing! Let us know if we can help in any way! |
Thanks, @stephendonner, it is my first PR to open source community. I will look at other open issues and try to fix some. |
Great; if you're interested, @sachs7 - would you mind going through http://firefox-test-engineering.readthedocs.io/en/latest/ and either filing Issues (https://github.com/mozilla/firefox-test-engineering/issues) or pull requests to fix anything obviously unclear/missing? |
@stephendonner going through the links which you shared above. |
|
||
import pytest | ||
|
||
from random import randrange | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was no need to move this line down. Both uuid
and random
are standard library modules, so according to PEP8 these should be grouped together. See https://www.python.org/dev/peps/pep-0008/#imports for more details.
Thanks @sachs7! So you know, there's no need to create new branches and open new pull requests after addressing review comments. You can simply add new commits to your existing branch and push it. GitHub will then update the open pull request with the new commits. |
Thanks, @davehunt. It was a good learning and I will improve on it. Thanks, for your suggestion and will keep it in mind. |