Roundup Robot added the comment:
New changeset ce99559efa46 by Chris Jerdonek in branch 'default':
Issue #16854: Fix regrtest.usage() regression introduced in 6e2e5adc0400.
http://hg.python.org/cpython/rev/ce99559efa46
--
___
Python tracker
Andrew Svetlov added the comment:
Good step!
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Python-bugs-list mailing list
Chris Jerdonek added the comment:
Updating patch after Benjamin's review.
In this new patch, in test_regrtest I now use the current, actual getopt code
to test and demonstrate backwards compatibility. Note that when I pasted the
code, I also fixed the three typos in the current getopt code
Chris Jerdonek added the comment:
Rietveld is erroring out on me again whenever I try to reply to a comment, so
I'm posting my comment here.
On 2012/12/27 18:29:22, Benjamin Peterson wrote:
On 2012/12/27 04:44:33, Benjamin Peterson wrote:
if val:
Again, we need this to match getopt
Roundup Robot added the comment:
New changeset 6e2e5adc0400 by Chris Jerdonek in branch 'default':
Issue #15302: Switch regrtest from using getopt to using argparse.
http://hg.python.org/cpython/rev/6e2e5adc0400
--
nosy: +python-dev
___
Python
Chris Jerdonek added the comment:
Thanks again for your reviews, Benjamin (and others).
I created issue 16799 for the next phase of this process: changing
regrtest.main() from operating on getopt-style parsed options to an argparse
Namespace object.
--
resolution: - fixed
stage:
Andrew Svetlov added the comment:
Anton, you are free to make that issue and propose a patch :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
Andrew Svetlov added the comment:
New patch is LGTM.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Python-bugs-list mailing list
Andrew Svetlov added the comment:
Chris, would you commit it?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Python-bugs-list
Chris Jerdonek added the comment:
BTW, maybe someone should create an issue for full tests coverage of regrtest?
I think such an issue would need to be a meta-issue because it is a very large
task (involving many patches) and may never be truly finished.
Chris, would you commit it?
Sure, I
Andrew Svetlov added the comment:
Agree with both points
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Python-bugs-list mailing
R. David Murray added the comment:
I'm still -0 on making a new file. regrtest will be running as __main__, so I
don't see any reason the test file can't import it to test functions within it.
--
___
Python tracker rep...@bugs.python.org
R. David Murray added the comment:
(I could, of course, be wrong :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Benjamin Peterson added the comment:
Yes, please don't make something called regrtestlib.py. Some of us like the
bash completetion to work with on a small prefix. :)
--
nosy: +benjamin.peterson
___
Python tracker rep...@bugs.python.org
Chris Jerdonek added the comment:
Benjamin, do you not want a new file at all, or are you just asking for a
different name? regrlib was the previous name unless you have another
suggestion.
--
___
Python tracker rep...@bugs.python.org
Benjamin Peterson added the comment:
I think bitdancer's suggestion of leaving it in regrtest.py is a good one.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
Chris Jerdonek added the comment:
I'm attaching an updated patch incorporating David and Benjamin's suggestion.
(Thanks a lot for the feedback, by the way.) I also added a test for the help
option and refactored to avoid having to use context managers to check
sys.argv, sys.stderr, etc.
Chris Jerdonek added the comment:
Here is an alternative patch with a cleaner diff (keeping the help-related
strings at the top before the import statements).
--
Added file: http://bugs.python.org/file28449/issue-15302-5.patch
___
Python tracker
Benjamin Peterson added the comment:
I posted some review comments.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Chris Jerdonek added the comment:
Thanks. I replied.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Python-bugs-list mailing list
Andrew Svetlov added the comment:
Well. Do you like to apply Chris patch and wait for next step appear?
On Mon, Dec 24, 2012 at 2:33 AM, R. David Murray rep...@bugs.python.org wrote:
R. David Murray added the comment:
I am -1 on doing this as one big refactoring, unless tests are written
Chris Jerdonek added the comment:
Do you like to apply Chris patch and wait for next step appear?
Just to clarify, I think this should read apply Chris patch after
updating/reviewing. A couple file renames are needed, and I noticed a typo in
a docstring. Other changes may be needed since 5
R. David Murray added the comment:
Yes, I apologize for not getting around to a review previously.
Chris: why regrlib.py at all? Why isn't the code in regrtest.py? And I'm not
clear on why there is a desire to have regrtest be a package. Did I miss that
discussion?
--
Chris Jerdonek added the comment:
why regrlib.py at all? Why isn't the code in regrtest.py?
It was for a few related reasons. It was primarily to make it easier to reason
about testing regrtest, and to avoid at the outset any pitfalls that might
arise from the circularity of regrtest
Chris Jerdonek added the comment:
By the way, I am in the process of cleaning up the patch a bit.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
Anton Kasyanov added the comment:
So I see that there is no need in full refactoring now.
BTW, maybe someone should create an issue for full tests coverage of regrtest?
Then it would be much easier to make a full refactoring.
--
___
Python tracker
Chris Jerdonek added the comment:
Here is an updated, improved patch.
--
Added file: http://bugs.python.org/file28422/issue-15302-3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
Anton Kasyanov added the comment:
I've looked through the second patch and I'm not sure about how argparse usage
was implemented here - parse_args() result is being converted to getopt-style
list of (option, value) pairs.
Is there any sense in using argparse this way?
--
nosy:
Chris Jerdonek added the comment:
The reason in part is because of the lack of unit tests of regrtest (as
commenters above have noted). By preserving the getopt interface, we can keep
almost all of the untested code as is.
You should view the patch as a first step in refactoring to use
Andrew Svetlov added the comment:
Hi Chris.
Today we had python sprint and I've guessed to Anton to refactor the patch in
good way with properly setting default values from regrtest.main to argparse
args. Then use proper argparse actions for manipulating that args.
After all we can use
Chris Jerdonek added the comment:
Yes, I agree with all of that but thought it would be easier to review if done
incrementally as separate steps. In any case, I will look for Anton's patch on
the review tool in case I have any comments.
--
___
R. David Murray added the comment:
I am -1 on doing this as one big refactoring, unless tests are written for
regrtest first. Incremental (over a period of weeks or months, so that the
changes get some soak time each time) is I think acceptable even without tests,
given that this is a test
Changes by Tshepang Lekhonkhobe tshep...@gmail.com:
--
nosy: +tshepang
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Georg Brandl ge...@python.org added the comment:
Please leave this for Python 3.4 -- it is not a bugfix.
--
nosy: +georg.brandl
versions: +Python 3.4 -Python 3.3
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
Here is a patch that creates some unit tests for the existing getopt argument
parsing code.
In response to the comments, I'm thinking of a less invasive approach that
involves wrapping argparse's parse_args() to return getopt-like
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
Attached is a first version of a complete patch.
Note that I found three bugs in the current argument parsing code in the course
of working on this patch: issue 15324, issue 15325, and issue 15326 (because of
various typos in the
Changes by Brett Cannon br...@python.org:
--
stage: needs patch - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
___
Senthil Kumaran sent...@uthcode.com added the comment:
On Mon, Jul 9, 2012 at 8:50 PM, Chris Jerdonek rep...@bugs.python.org wrote:
Sure, if someone is open to reviewing it.
Yes, please go ahead. I can review it and I believe others should be
able to as well. As a first short, I think, it may
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
Thanks, Senthil. That is my plan. I should be able to have code with tests in
no later than a week.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
By the way, issue 15300 has a related patch that is ready to review today.
Assuming that one is okay, it would make sense to commit first because it
overlaps with the changes I'll be doing here.
Issue 15305 is another related issue
R. David Murray rdmur...@bitdance.com added the comment:
As mentioned, the first step is to create some tests that can validate the
current behavior, so that changes don't break things. This is a non-trivial
task. I know from experience with a similar refactoring that even seemingly
simple
Ezio Melotti ezio.melo...@gmail.com added the comment:
Do you want to propose a patch?
regrtest has no tests, so switching to argparse might cause more harm than good
right now.
--
nosy: +ezio.melotti
stage: - needs patch
type: - enhancement
___
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
Sure, if someone is open to reviewing it.
The parsing code doesn't seem to be doing anything too fancy right now. I can
decouple the parsing code and begin adding tests around parts that may need it
more. Increasing coverage will be
New submission from Chris Jerdonek chris.jerdo...@gmail.com:
I think it would be an improvement to switch from using getopt to argparse in
test.regrtest. The code would be easier to maintain, it would give us more
powerful options going forward, and it would improve the usability of the test
Chris Jerdonek chris.jerdo...@gmail.com added the comment:
It is also in the spirit of dogfooding.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15302
___
45 matches
Mail list logo
