Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

Adding sudo args #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding sudo args #75

wants to merge 1 commit into from

Conversation

Erf-
Copy link

@Erf- Erf- commented Jul 27, 2015

As you can see I splited my previous pull request. Then this one only implement the adding of 'sudo' args in terminal calls and the related tests. Note there is one failure in the tests i did not succeed to solve yet.

kwargs = {'stderr':subprocess.STDOUT}
scheme = Scheme('wlan0', 'test')
subprocess.check_output = MagicMock(return_value=SUCCESSFUL_IFUP_OUTPUT)
scheme.activate(True)
Copy link
Owner

Choose a reason for hiding this comment

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

Can you write this as scheme.activate(sudo=True). I want the recommended interface to use the keyword argument instead of the kwargs.

Copy link
Author

Choose a reason for hiding this comment

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

should I do the same for cell.all in scan.py?

@Erf- Erf- force-pushed the adding-sudo-args branch 7 times, most recently from 7579355 to 1c2775b Compare August 3, 2015 09:45
@@ -278,3 +282,15 @@ def test_scanning(self):
Encryption key:off
Bit Rates:144 Mb/s
"""

output = string.join([IWLIST_SCAN_NO_ENCRYPTION,

Choose a reason for hiding this comment

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

" ".join() should work, not need for importing string

@yohanboniface
Copy link

Please also revert the file mode changes.

@Erf- Erf- force-pushed the adding-sudo-args branch 9 times, most recently from d1d423f to 973de9b Compare August 3, 2015 17:18
@Erf-
Copy link
Author

Erf- commented Aug 3, 2015

Hi Rocky. I tried to match what you and Yohan Boniface told me to do. Then can you check my latest version.
PS: Can you also delete the other pull requests I did. Sorry I am not used to deal with the Travis tool so I did such a mess...

@@ -13,8 +13,8 @@ def read(fname):


install_requires = [
'setuptools',
Copy link
Owner

Choose a reason for hiding this comment

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

why did you remove this?

@rockymeza
Copy link
Owner

Ok, I had just a few comments. After those are resolved let's merge this baby. Thanks for all the hard work @Erf- and thanks for the extra review help @yohanboniface.

Add sudo args to terminal calls.
	modified :         tests/test_parsing.py
	modified :         tests/test_scan.py
	modified :         tests/test_schemes.py
	removed :        wifi/dummydata.py
	modified :         wifi/scheme.py

Pull request fix #1.
	modifié :         tests/test_scan.py
	modifié :         tests/test_schemes.py

Pull request fix #2.
	modifié:         tests/test_scan.py
	modifié:         tests/test_schemes.py

Pull request fix rockymeza#3.
	modifié:         setup.py
Add 'mock' to the requires.

Pull request fix rockymeza#4.
	modifié:         .travis.yml
Demand for latest update of setuptools when installing.

Pull request fix rockymeza#5.
Improve code readability.

Add Travis to my own repository.

Restore file permissions.

Pull request fixes rebase.

Rebase of the pull request fixes in order to clear the code and to solve build issues.
@Erf-
Copy link
Author

Erf- commented Aug 4, 2015

I made the review in a commit amend to avoid polluting the pull request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants