#18 modlint: Use asynchronous requests using python-tornado to query Fedora dist-git when checking commit ids.
Closed by nphilipp. Opened by jkaluza.
jkaluza/modulemd master  into  master

Download 18.patch

It uses python-tornado - I'm not sure if that's the right module to use. I'm OK to rewrite that to something different, but we should discuss what we should use for async http requests.

:thumbsup:

rebased

What's the reason for the double leading underscore in this method? Normally, this should only used to avoid name clashes in subclasses. I assume that _schedule_requests() has only one because name-mangling bit you in the nested wrapped() function, right? :wink:

Is there a problem with simply using print() here?

https://fedoraproject.org/wiki/Modularity/Development/Coding_Style#Comments_and_Docstrings :stuck_out_tongue:

NB: these are "positional" arguments (or just "arguments"). Only arguments that have a default value are called "keyword arguments" (or colloquially if you refer to positional arguments "by name" when calling the function/method).

Error status as the return value? That's too C-ish for me ;). And catching KeyboardInterrupt should normally go to the main function or block.

...and here the error status isn't looked at :wink: Wrapping the collected errors above in a new exception and catching it here looks more natural to me. :smiley:

rebased

  • I commented above on the doc comments you used, is it intentional that you don't use Sphinx-format docstrings?
  • I didn't mean to get rid of KeyboardInterrupt, just move it into the main block
  • Some of the lines are too long (PEP8), can you please fix that? Also, the commit log subject line is too long, too, should be shorter than ~50 characters (that's a guideline, some things can't be expressed well in such little space, the "hard limit" here would be 80)

rebased

Fixed the remaining long lines (one in each code and commit log), and merged in commit 98550b68ed33b886dfebe6230f2341e50e498868.

Pull-Request has been closed by nphilipp

Metadata