You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/03/26 22:04:39 UTC
[GitHub] [zookeeper] TisonKun opened a new pull request #1295:
ZOOKEEPER-3771: Update zk-merge-pr script to Python3
TisonKun opened a new pull request #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295
@eolivelli generally I use `2to3` util and check the codepath that I can arrive, manually fix some lines.
But it seems we can verify this patch totally when merging this patch :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] TisonKun commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
TisonKun commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-606260388
@nkalmar Thanks for your review and test. That is the only `encode/decode` pair on `str`. I pushed a follow-up commit to address it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] nkalmar edited a comment on issue #1295:
ZOOKEEPER-3771: Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
nkalmar edited a comment on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605966459
I voted for v3 only on the mail list, but I agree we could keep v2 for a time, but maybe add a warning when script starts that it is deprecated?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605485849
I agree with @TisonKun
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
nkalmar commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-606575068
Merged to master and branch-3.6 (branch-3.5 does not have the script in the repo). Thanks @TisonKun !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] anmolnar commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
anmolnar commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605458771
Great work @TisonKun !
@eolivelli Why do you want to keep the python2 script? Source control does it for you for free.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-604996501
can we keep the original file and add a new zk-merge-pr.py3 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] enixon commented on issue #1295: ZOOKEEPER-3771: Update
zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
enixon commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-606278771
This looks good to me - super happy for py3 support.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-604718122
I will be happy to try out this patch for my next use of the script
thank you for your quick response !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] asfgit closed pull request #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] TisonKun commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
TisonKun commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605023451
@eolivelli i'd prefer a `py2` and use py3 as `py` default.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] nkalmar commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
nkalmar commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605966459
I voted for v2 only on the mail list, but I agree we could keep v2 for a time, but maybe add a warning when script start that it is deprecated?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] nkalmar edited a comment on issue #1295:
ZOOKEEPER-3771: Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
nkalmar edited a comment on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605966459
I voted for v3 only on the mail list, but I agree we could keep v2 for a time, but maybe add a warning when script start that it is deprecated?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] TisonKun commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
TisonKun commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605467575
Hi @anmolnar ! Here is part of the original mail
>AFAIK python2 is EOL so maybe we can just have only python3 on master
and leave the old version on git repo
I can see one of the reasons we still have a py2 script is that possibly some of our committers still use py2 so that they can easily use the py2 script which keeps previous workflow. Though, a community lazy agreement might help on dropping it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zookeeper] eolivelli commented on issue #1295: ZOOKEEPER-3771:
Update zk-merge-pr script to Python3
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1295: ZOOKEEPER-3771: Update zk-merge-pr script to Python3
URL: https://github.com/apache/zookeeper/pull/1295#issuecomment-605027211
Ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services