You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2015/04/17 15:17:38 UTC
Review Request 33304: PROTON-490: Run 'futurize' conversion tool over
proton-c/bindings
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/
-----------------------------------------------------------
Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
Bugs: PROTON-490
https://issues.apache.org/jira/browse/PROTON-490
Repository: qpid-proton-git
Description
-------
First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
Diffs
-----
proton-c/bindings/python/proton/__init__.py eeb7768
proton-c/bindings/python/proton/reactor.py 467bb76
proton-c/bindings/python/proton/utils.py b5ecdf7
Diff: https://reviews.apache.org/r/33304/diff/
Testing
-------
Ran the unit tests - all pass including Jython-based tests.
Thanks,
Kenneth Giusti
Re: Review Request 33304: PROTON-490: Run 'futurize' conversion tool
over proton-c/bindings
Posted by Rafael Schloming <rh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/#review80464
-----------------------------------------------------------
proton-c/bindings/python/proton/reactor.py
<https://reviews.apache.org/r/33304/#comment130374>
This next stuff looks suspicious to me, porticularly if this is indeed a mechanical change resulting from the 2to3 tool.
Is this really supposed to recursively call itself in this way?
- Rafael Schloming
On April 17, 2015, 1:17 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33304/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 1:17 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
>
>
> Bugs: PROTON-490
> https://issues.apache.org/jira/browse/PROTON-490
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
>
> These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/__init__.py eeb7768
> proton-c/bindings/python/proton/reactor.py 467bb76
> proton-c/bindings/python/proton/utils.py b5ecdf7
>
> Diff: https://reviews.apache.org/r/33304/diff/
>
>
> Testing
> -------
>
> Ran the unit tests - all pass including Jython-based tests.
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 33304: PROTON-490: Run 'futurize' conversion tool
over proton-c/bindings
Posted by Rafael Schloming <rh...@apache.org>.
> On April 17, 2015, 5:35 p.m., Kenneth Giusti wrote:
> > proton-c/bindings/python/proton/reactor.py, line 564
> > <https://reviews.apache.org/r/33304/diff/1/?file=933122#file933122line564>
> >
> > It's not calling next on itself, rather it calls next on the contained iterator (to the self.values array).
> >
> > Agreed that it looks a bit weird, but the indent appears to be a 'circular iterator' - one that never throws StopIteration but returns to the sequence start instead.
> >
> > At least that's how it works when I run it by hand.
Gotcha, I actually misread the diff. I thought the "def next()" was top level so it was literally recursively calling itself. I see it's actually a method, so it's not recursive the way I thought it was. Apologies for the false alarm.
- Rafael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/#review80484
-----------------------------------------------------------
On April 17, 2015, 1:17 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33304/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 1:17 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
>
>
> Bugs: PROTON-490
> https://issues.apache.org/jira/browse/PROTON-490
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
>
> These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/__init__.py eeb7768
> proton-c/bindings/python/proton/reactor.py 467bb76
> proton-c/bindings/python/proton/utils.py b5ecdf7
>
> Diff: https://reviews.apache.org/r/33304/diff/
>
>
> Testing
> -------
>
> Ran the unit tests - all pass including Jython-based tests.
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 33304: PROTON-490: Run 'futurize' conversion tool
over proton-c/bindings
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/#review80484
-----------------------------------------------------------
proton-c/bindings/python/proton/reactor.py
<https://reviews.apache.org/r/33304/#comment130401>
It's not calling next on itself, rather it calls next on the contained iterator (to the self.values array).
Agreed that it looks a bit weird, but the indent appears to be a 'circular iterator' - one that never throws StopIteration but returns to the sequence start instead.
At least that's how it works when I run it by hand.
- Kenneth Giusti
On April 17, 2015, 1:17 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33304/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 1:17 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
>
>
> Bugs: PROTON-490
> https://issues.apache.org/jira/browse/PROTON-490
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
>
> These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/__init__.py eeb7768
> proton-c/bindings/python/proton/reactor.py 467bb76
> proton-c/bindings/python/proton/utils.py b5ecdf7
>
> Diff: https://reviews.apache.org/r/33304/diff/
>
>
> Testing
> -------
>
> Ran the unit tests - all pass including Jython-based tests.
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 33304: PROTON-490: Run 'futurize' conversion tool
over proton-c/bindings
Posted by Alan Conway <ac...@redhat.com>.
> On April 17, 2015, 2:16 p.m., Alan Conway wrote:
> > Looks good (surprised there wasn't more). Run the entire test set before committing, we have bits of python in surprising places so best be sure.
Actually you shoulld run demos too. We badly, badly need to automate the demos.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/#review80455
-----------------------------------------------------------
On April 17, 2015, 1:17 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33304/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 1:17 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
>
>
> Bugs: PROTON-490
> https://issues.apache.org/jira/browse/PROTON-490
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
>
> These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/__init__.py eeb7768
> proton-c/bindings/python/proton/reactor.py 467bb76
> proton-c/bindings/python/proton/utils.py b5ecdf7
>
> Diff: https://reviews.apache.org/r/33304/diff/
>
>
> Testing
> -------
>
> Ran the unit tests - all pass including Jython-based tests.
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 33304: PROTON-490: Run 'futurize' conversion tool
over proton-c/bindings
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33304/#review80455
-----------------------------------------------------------
Ship it!
Looks good (surprised there wasn't more). Run the entire test set before committing, we have bits of python in surprising places so best be sure.
- Alan Conway
On April 17, 2015, 1:17 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33304/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 1:17 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, Rafael Schloming, and Robbie Gemmell.
>
>
> Bugs: PROTON-490
> https://issues.apache.org/jira/browse/PROTON-490
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> First of a series of patches, this patch shows the changes made by the futurize tool when run on the proton-c/bindings directory.
>
> These changes preserve python2 compatibility while avoiding language features deprecated by python3. These changes don't enable python3 support, they merely eliminate the syntax/features that won't port.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/__init__.py eeb7768
> proton-c/bindings/python/proton/reactor.py 467bb76
> proton-c/bindings/python/proton/utils.py b5ecdf7
>
> Diff: https://reviews.apache.org/r/33304/diff/
>
>
> Testing
> -------
>
> Ran the unit tests - all pass including Jython-based tests.
>
>
> Thanks,
>
> Kenneth Giusti
>
>