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

Improve documentation in tools submodule #212

Closed
embolalia opened this issue Mar 18, 2013 · 3 comments
Closed

Improve documentation in tools submodule #212

embolalia opened this issue Mar 18, 2013 · 3 comments

Comments

@embolalia
Copy link
Contributor

For example, stderr is just documented "print to stderr". I'm not sure whether that function should be publicly visible, either. Many of these I think are things @elad661 wrote, so maybe he could help out with this.

@elad661
Copy link
Contributor

elad661 commented Mar 18, 2013

stderr just looks better than sys.stderr.write or print >>sys.stderr,

On Mar 18, 2013 11:15 PM, "Edward powell" notifications@github.com wrote:

For example, stderr is just documented "print to stderr". I'm not sure
whether that function should be publicly visible, either. Many of these I
think are things @elad661 https://github.com/elad661 wrote, so maybe he
could help out with this.


Reply to this email directly or view it on GitHubhttps://github.com//issues/212
.

@embolalia
Copy link
Contributor Author

I don't think that's really necessary. Sure it's neater, but it's not used very often and, when it is, it's within the core. Modules certainly shouldn't be printing to stderr, so that should probably be removed from the public-facing API. Certainly, stdout is unnecessary.

Other issues in that file:

  • I don't see anywhere that the OutputRedirect object is actually used, so that can likely be removed as well.
  • check_pid is also unused, and if it were it should definitely say that it kills the process, it doesn't just check that it exists.
  • The deprecated decorator, while useful, probably ought to be kept private. For now, though, we'll just leave it be, and remove it in 4.0 along with the functions it's used on.
  • I don't know what verify_ssl_cn does, but since it only seems to be used once in irc.py, and nowhere else, it should probably be moved there.
  • Ddict's doc is vague out of context.
  • The docstring for the file is the copyright, and not really descriptive of the file's purpose.

I don't know what all of this ought to be left until the next major version, since some of these would technically be breaking changes, even if tools was only just made to show up in the html documentation. I also wonder if we should combine tools and web in 4.0, since they're mostly for the same purpose...

@elad661
Copy link
Contributor

elad661 commented Mar 19, 2013

OutputRedirect is used in willie.py, it makes it possible to write stdio.log

on linux/posix system, "killing" a process with signal number 0 doesn't do anything, but raises OSError if the process doesn't exist. Google it and check os.kill docs. check_pid is also used in willie.py and there are usecases for using it in modules as well.

verify_ssl_cn does exactly what its name says: it verifies a cert cn (common name) to see if it matches, ie. if you try to connect to google.com you'd want the cn to be google.com and not maliciouswebsite.org. This is a security mechanism and modules that require secure connection might want to use it in the future to make sure there was no man-in-the-middle attack or DNS poisoning

Ddict is a simple helper to create multi-dimensional dicts.

embolalia added a commit that referenced this issue Mar 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants