Re: Coding Style
I'm not picking on Josef here - I'm sure I've submitted code recently with lint errors, this was just the review I was looking at which triggered the idea: https://phab.qadevel.cloud.fedoraproject.org/D389 No worries, I'm not taking it personaly. As I commented in the D389 - the not compliant parts of the code were mostly in the spirit ofthe rest of the code in the respective files (thus actually honoring the PEP8 -https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds ). Not saying that it is the best though. exceptions that we'd want, I'm proposing that we use strict PEP8 with almost no exceptions. For me, strict PEP8 is next-to-unusable, and almost always leads to code like this: +result = self.resultsdb.create_result(job_id=job_data['id'], + testcase_name=checkname, + outcome=detail.outcome, + summary=detail.summary + or None, + log_url=result_log_url, + item=detail.item, + type=detail.report_type, + **detail.keyvals + ) Hard to read, and heavily concentrated to the right edge of the 80-char mark. ... In this case it would involve asking Josef to stop putting spaces between parameter keyvals I actually did stop doing that quite some time ago :) First of all I'd suggest to move our codebase to strict PEP8 (or as-strict-as-possible), so we can have see how our code looks like, when PEP8 compliant. For starters, we could just plain use autopep8 - https://pypi.python.org/pypi/autopep8/ How about that? J. ___ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
2015-06-15 Fedora QA Devel Meeting Minutes
= #fedora-meeting-1: fedora-qadevel = Minutes: http://meetbot.fedoraproject.org/fedora-meeting-1/2015-06-15/fedora-qadevel.2015-06-15-14.02.html Minutes (text): http://meetbot.fedoraproject.org/fedora-meeting-1/2015-06-15/fedora-qadevel.2015-06-15-14.02.txt Log: http://meetbot.fedoraproject.org/fedora-meeting-1/2015-06-15/fedora-qadevel.2015-06-15-14.02.log.html Meeting summary --- * Roll Call (tflink, 14:02:55) * mkrizek status report (mkrizek, 14:04:32) * sent a patch changing log locations (mkrizek, 14:04:46) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T495 (mkrizek, 14:04:46) * LINK: https://phab.qadevel.cloud.fedoraproject.org/D382 (mkrizek, 14:04:46) * sent a patch adding distgit directive (mkrizek, 14:04:46) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T492 (mkrizek, 14:04:46) * LINK: https://phab.qadevel.cloud.fedoraproject.org/D383 (mkrizek, 14:04:49) * sent a patch adding fedmenu to blockerbugs (mkrizek, 14:04:51) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T466 (mkrizek, 14:04:54) * LINK: https://phab.qadevel.cloud.fedoraproject.org/D388 (mkrizek, 14:04:56) * sent patch fixing Provided Variables in docs (mkrizek, 14:04:59) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T497 (mkrizek, 14:05:01) * LINK: https://phab.qadevel.cloud.fedoraproject.org/D391 (mkrizek, 14:05:04) * created a ticket for sorting artifacts date directories (mkrizek, 14:05:06) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T498 (mkrizek, 14:05:09) * still have some open reviews to be completed (mkrizek, 14:05:11) * kparal status report (kparal, 14:08:25) * lots of reviews (kparal, 14:08:32) * proposed config/cli/formula changes wrt disposable clients (kparal, 14:08:36) * LINK: https://phab.qadevel.cloud.fedoraproject.org/T408 (kparal, 14:08:36) * investigated nested virt and whether it would work for our purposes (part of T408) (kparal, 14:08:42) * jskladan status update (jskladan, 14:10:59) * T468/D387 depcheck: create per-build and per-update artifacts (will merge today) (jskladan, 14:10:59) * T470/D389 allow checks to provide URLs for their specific per-build/update logs from artifactsdir (provided hopefuly passable version today) (jskladan, 14:10:59) * PEP8-compliant-ish libtaskotron (jskladan, 14:10:59) * LINK: https://phab.qadevel.cloud.fedoraproject.org/differential/diff/1036/ (jskladan, 14:10:59) * status report - tflink (tflink, 14:19:01) * more futzing with beaker.stg, networking issues appear to be worked out for now (tflink, 14:19:01) * slight progress on testCloud, waiting for review, should have a new release soon (tflink, 14:19:01) * a few reviews, started to do some overdue ticket cleanup (tflink, 14:19:01) * looking into running taskotron with arm clients (tflink, 14:19:01) * planning - code style (tflink, 14:30:20) * jskladan posted a first runthrough of autopep8 on the libtaskotron codebase: https://phab.qadevel.cloud.fedoraproject.org/differential/diff/1036/ (tflink, 14:30:53) * the main idea is to have a coding standard and not spend eternity debating over what set of rules is best (tflink, 14:32:14) * LINK: https://phab.qadevel.cloud.fedoraproject.org/file/data/yar3gaju7rtjcsn33bjk/PHID-FILE-lvpsyhkk3czoi2t54zu3/D389_rdb-directive_lintfixes.diff (kparal, 14:52:33) * farther discussion on qadevel@, hopefully not a debate until the end of known time :) (tflink, 14:57:06) * planning - tasks (tflink, 14:58:13) * pester tflink if anyone runs out of tasks to do :) (tflink, 15:03:04) * open floor (tflink, 15:03:21) Meeting ended at 15:04:20 UTC. Action Items Action Items, by person --- * **UNASSIGNED** * (none) People Present (lines said) --- * tflink (99) * kparal (62) * jskladan (33) * mkrizek (30) * pwhalen (9) * zodbot (4) Generated by `MeetBot`_ 0.1.4 .. _`MeetBot`: http://wiki.debian.org/MeetBot pgpTDR9pbO5ma.pgp Description: OpenPGP digital signature ___ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
Re: Coding Style
First of all I'd suggest to move our codebase to strict PEP8 (or as-strict-as-possible), so we can have see how our code looks like, when PEP8 compliant. For starters, we could just plain use autopep8 - https://pypi.python.org/pypi/autopep8/ How about that? For the record, Josef put up the diff here: https://phab.qadevel.cloud.fedoraproject.org/differential/diff/1036/ As the most vocal naysayer during the IRC meeting, I have went through all of those changes (and I have read PEP8 again), and I have to say the changes are not that bad. I have feared that the code would look similarly to what Josef quoted in the email, but PEP8 is actually saner than I expected and allows much better stylistic formatting. Which in fact touches one of the concerns I have, that it's hard to realize how to write the code pretty and still comply to PEP8, but it's easy to realize how to write the code ugly and comply. At least that's my experience. I still have a few pet peeves about PEP8, for example check.py:189-192 or config_defaults.py:63. I think human being can use whitespace to make code much more readable (the code is in a monospace font for a reason), and PEP8's I know better approach of enforced conformity quite bothers me. But I understand that can be highly subjective, and for some people consistency is above readability, for me it is the other way around. That said, if everyone else happily supports this idea, I don't object against a trial run in which we drop all custom configuration from .arclint and enable pep8 checking. We will see how it works out. I don't think it will boost our productivity, I think it will just move the wasted time from reviews (let's admit it, almost none in the past months) to the patch preparation process and multiply it, but maybe I'm wrong. PS: Speaking of line length, PEP8 actually allows going up to 100 chars [1], and most of our existing codebase meets that, so I don't see that as much of a problem. The comments and docstrings still need to be wrapped at 72 chars, though, so I'm curious to see if we start to enforce that. [1] https://www.python.org/dev/peps/pep-0008/#maximum-line-length ___ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
Re: Coding Style
On Mon, 2015-06-15 at 06:19 -0400, Josef Skladanka wrote: I'm not picking on Josef here - I'm sure I've submitted code recently with lint errors, this was just the review I was looking at which triggered the idea: https://phab.qadevel.cloud.fedoraproject.org/D389 No worries, I'm not taking it personaly. As I commented in the D389 - the not compliant parts of the code were mostly in the spirit ofthe rest of the code in the respective files (thus actually honoring the PEP8 -https://www.python.org/dev/peps/pep-0008/#a-foolish-consistency -is-the-hobgoblin-of-little-minds ). Not saying that it is the best though. exceptions that we'd want, I'm proposing that we use strict PEP8 with almost no exceptions. For me, strict PEP8 is next-to-unusable, and almost always leads to code like this: +result = self.resultsdb.create_result(job_id=job_data['id'], + testcase_name=checkname, + outcome=detail.outcome, + summary=detail.summary + or None, + log_url=result_log_url, + item=detail.item, + type=detail.report_type, + **detail.keyvals + ) As I read it (of course IMBW), PEP-8 also allows for: result = self.resultsdb.create_result( job_id=job_data['id'], testcase_name=checkname, outcome=detail.outcome, summary=detail.summary or None, log_url=result_log_url, item=detail.item, type=detail.report_type, **detail.keyvals) unless the next line would be indented, in which case you add one extra level of indentation here. I use that style quite a bit in my stuff. I do think it makes sense to be a bit less strict than PEP-8 for line lengths especially when we have existing code that has been written to longer lengths, though. -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net ___ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
