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

allow user-spcified network interface for instance deployment, fixes … #22

Merged
merged 3 commits into from May 24, 2019

Conversation

VertexC
Copy link
Contributor

@VertexC VertexC commented May 19, 2019

@pep8speaks
Copy link

pep8speaks commented May 19, 2019

Hello @VertexC, Thank you for updating the PR!

Great work! I discovered no PEP8 issues in this Pull Request. 🥇

Comment last updated at 2019-05-24 08:27:19 UTC

@canihavesomecoffee
Copy link
Member

I've never seen the approach of using the record function to copy over the app configuration. Usually I import the app from the main, and access the configuration directly.

Does this (seemingly more complex) method have an advantage over what I'm usually doing? :)

@VertexC
Copy link
Contributor Author

VertexC commented May 24, 2019

It is indeed easier and more neat to do from pipot.run import app. While it introduces circular import between pipot.run and mod_honeypot module, which seems not a good practice.

@VertexC VertexC force-pushed the interface branch 3 times, most recently from 774468b to 67a4505 Compare May 24, 2019 08:26
@canihavesomecoffee
Copy link
Member

@pep8speaks suggest diff

@pep8speaks
Copy link

Here you go with the gist !

You can ask me to create a PR against this branch with those fixes. Simply comment @pep8speaks pep8ify.

@canihavesomecoffee @VertexC

@canihavesomecoffee canihavesomecoffee merged commit e55af81 into PiPot:master May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants