Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sunday 19 January 2014 17:46:36 Alec Warner wrote: On Sun, Jan 19, 2014 at 2:45 PM, Alexander Berntsen wrote: On 19/01/14 22:22, Sebastian Luther wrote: The usual doc string style used in portage is: text Please use that for new functions. Also make sure you don't use spaces to indent the last . As mentioned by Mike in another thread, we should use PEP 257[0]. I will convert old code to conform to this... sometime... soon... (I promise!) So if new patches could just do that right away, that would be neat. Does pylint or pyflakes point out if you mess it up? Automation for the win. the good news is that i wrote a pylintrc module for Chromium OS to enforce docstring style. the bad news is that it doesn't work with pyflakes. https://chromium.googlesource.com/chromiumos/chromite/+/master/cros/commands/lint.py what we did there was just merge it and then have people fix things up as they went rather than try to clean it all up first. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
Am 19.01.2014 04:07, schrieb W. Trevor King: I felt like I should actually make a useful contribution to balance the less useful commit-message discussion ;). Browsing through the open Portage bugs, #175612 looked interesting. After I looked at pym/portage/package/ebuild/fetch.py, I decided to take a step back and just try and refactor fetch(), which was pushing 1k lines. Here are three paches pulling fairly self-contained blocks out of fetch(). I thought I'd float them to the list to see if this was a productive avenue, or if this is going to be too much work to review. I tried to avoid making too many changes other than the function-extraction, but in some places I couldn't help myself ;). Good idea. The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. The tests can be run as non-root user. Make sure all the files in the git repo are +rw for that user. You should check the result using pyflakes. Do you have these patches in the publicly accessible git repo? Cheers, Trevor [1]: https://bugs.gentoo.org/show_bug.cgi?id=175612 W. Trevor King (3): pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size pym/portage/package/ebuild/fetch.py: Factor out _get_uris pym/portage/package/ebuild/fetch.py | 305 +--- 1 file changed, 177 insertions(+), 128 deletions(-)
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 09:05:41PM +0100, Sebastian Luther wrote: Am 19.01.2014 04:07, schrieb W. Trevor King: The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. The tests can be run as non-root user. Make sure all the files in the git repo are +rw for that user. Ok. I'll go back and check the results on master so I can compare. You should check the result using pyflakes. Thanks. This picked up a few errors in the third patch. I'll post v2 shortly. Do you have these patches in the publicly accessible git repo? I do now: git://tremily.us/portage.git fetch-refactor Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
Am 19.01.2014 04:07, schrieb W. Trevor King: The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. Since I doubt there's currently any test or any way to write one, I'd suggest to try and run emerge and analyze coverage with dev-python/coverage or a similar tool.
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 22:22, Sebastian Luther wrote: The usual doc string style used in portage is: text Please use that for new functions. Also make sure you don't use spaces to indent the last . As mentioned by Mike in another thread, we should use PEP 257[0]. I will convert old code to conform to this... sometime... soon... (I promise!) So if new patches could just do that right away, that would be neat. [0] http://www.python.org/dev/peps/pep-0257/ - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVaAACgkQRtClrXBQc7XjTwD/UsCQo+SJdiCX/QsMC8jFDPic k0jEAN6DA5xML6/nJYQA/36FszMfVMZ/vzg5VF9FS6BDWwGm/Qy2whyqLiF3FOrX =+5Yy -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 2:45 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 22:22, Sebastian Luther wrote: The usual doc string style used in portage is: text Please use that for new functions. Also make sure you don't use spaces to indent the last . As mentioned by Mike in another thread, we should use PEP 257[0]. I will convert old code to conform to this... sometime... soon... (I promise!) So if new patches could just do that right away, that would be neat. Does pylint or pyflakes point out if you mess it up? Automation for the win. -A [0] http://www.python.org/dev/peps/pep-0257/ - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVaAACgkQRtClrXBQc7XjTwD/UsCQo+SJdiCX/QsMC8jFDPic k0jEAN6DA5xML6/nJYQA/36FszMfVMZ/vzg5VF9FS6BDWwGm/Qy2whyqLiF3FOrX =+5Yy -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:46, Alec Warner wrote: Does pylint or pyflakes point out if you mess it up? Not very successfully in my experience. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVrsACgkQRtClrXBQc7XZcQD9GD8lV8HpZbQjpqu8ggNh4vOo /fvFsCI2PaWiEsC4ISUA/iYRiXAYD7SEF4a3R/t6vOdmEQSHyvZcxH9NDwI4/biQ =RMJ0 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 2:50 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:46, Alec Warner wrote: Does pylint or pyflakes point out if you mess it up? Not very successfully in my experience. I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) -A - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcVrsACgkQRtClrXBQc7XZcQD9GD8lV8HpZbQjpqu8ggNh4vOo /fvFsCI2PaWiEsC4ISUA/iYRiXAYD7SEF4a3R/t6vOdmEQSHyvZcxH9NDwI4/biQ =RMJ0 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:54, Alec Warner wrote: I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) Feel free to write a tool for this, or to contribute to any of the numerous linters and/or editor plug-ins. It would be much appreciated. As for the difficulty of PEP 257... I have higher hopes for Portage contributors than getting stuck at that. If I write a patch that makes most of the docstrings follow it, then they can infer 99% of the extra rules by just looking at the other functions and methods. If they fail to comply, we can just mention it. If the docstring formatting is the biggest issue with their patch, I doubt they'll have a hard time fixing it. - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcZPQACgkQRtClrXBQc7XizAD/Y/Gxc7N6VkgNFWRgP5lmQ84r UwSne2xaqJYphY9x1TcBAIRpjBHB580edLz/8zpT14lqhW3oOmeMz0pNMB8ssW5d =+zp5 -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
On Sun, Jan 19, 2014 at 3:51 PM, Alexander Berntsen alexan...@plaimi.netwrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/01/14 23:54, Alec Warner wrote: I'm very against add a bunch of extra rules that have to be enforced by hand. I want to make it easy to contribute, not more difficult. If bob can run a tool that tells him all the things that are wrong with his patch, that avoids us having like 1/3rd of the conversations on list ;) Feel free to write a tool for this, or to contribute to any of the numerous linters and/or editor plug-ins. It would be much appreciated. I already prefer pylint, and I think it does cover most of what I want. I am working a pylintrc patch. As for the difficulty of PEP 257... I have higher hopes for Portage contributors than getting stuck at that. If I write a patch that makes most of the docstrings follow it, then they can infer 99% of the extra rules by just looking at the other functions and methods. If they fail to comply, we can just mention it. If the docstring formatting is the biggest issue with their patch, I doubt they'll have a hard time fixing it. I'm not saying its hard, I'm saying it is a giant waste of time for the list to tell people 'hey your docstrings are wrong' when they can just run a tool to do it ;) -A - -- Alexander alexan...@plaimi.net http://plaimi.net/~alexander -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlLcZPQACgkQRtClrXBQc7XizAD/Y/Gxc7N6VkgNFWRgP5lmQ84r UwSne2xaqJYphY9x1TcBAIRpjBHB580edLz/8zpT14lqhW3oOmeMz0pNMB8ssW5d =+zp5 -END PGP SIGNATURE-
[gentoo-portage-dev] [PATCH 0/3] Initial fetch() refactoring
I felt like I should actually make a useful contribution to balance the less useful commit-message discussion ;). Browsing through the open Portage bugs, #175612 looked interesting. After I looked at pym/portage/package/ebuild/fetch.py, I decided to take a step back and just try and refactor fetch(), which was pushing 1k lines. Here are three paches pulling fairly self-contained blocks out of fetch(). I thought I'd float them to the list to see if this was a productive avenue, or if this is going to be too much work to review. I tried to avoid making too many changes other than the function-extraction, but in some places I couldn't help myself ;). The patches aren't particularly well tested yet. I ran the test suite and got some errors, but they seemed to be related to my non-root invocation, and not due to these changes themselves. I thought I'd post my work so far, before digging deeper into the test suite. Cheers, Trevor [1]: https://bugs.gentoo.org/show_bug.cgi?id=175612 W. Trevor King (3): pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries pym/portage/package/ebuild/fetch.py: Factor out _get_fetch_resume_size pym/portage/package/ebuild/fetch.py: Factor out _get_uris pym/portage/package/ebuild/fetch.py | 305 +--- 1 file changed, 177 insertions(+), 128 deletions(-) -- 1.8.5.2.8.g0f6c0d1
