[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 @Randgalt Exactly. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user Randgalt commented on the issue: https://github.com/apache/zookeeper/pull/122 OK - to be clear, I have PR #151 which applies the legacy API to branch-3.5. Then, I'll rework _this_ PR (against master) to only have the Admin API, right? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 >> Do we need a separate issue for the 3.6 change? No we don't (what @fpj prefers wrapping both PR in one shot under ZOOKEEPER-2642). @Randgalt Do you mind update the current PR (https://github.com/apache/zookeeper/pull/122) so it apply to master, by removing the old reconfig APIs? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user Randgalt commented on the issue: https://github.com/apache/zookeeper/pull/122 Do we need a separate issue for the 3.6 change? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user Randgalt commented on the issue: https://github.com/apache/zookeeper/pull/122 See #151 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user Randgalt commented on the issue: https://github.com/apache/zookeeper/pull/122 Will do --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 @hanm @Randgalt sounds like a plan, let's follow the steps that Michael lined up above. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 >> Breaking changes should be in a 3.6 version. Sounds OK to me. >> We need to either create a new issue or have a different pull request for 3.5 under this same issue I think this pull request (122) should target branch 3.5, because it keeps the old API. We need create a new PR that target master which only contain updates to the API name (reconfig -> reconfigure) and some doc / test updates without bringing back the old APIs. @Randgalt Could you update the base branch of this PR to be branch-3.5 instead of master? I think this PR is ready to land in branch-3.5. @fpj I can create a new PR target master that only contain API name changes, if @Randgalt does not get ahead of me on this work. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user Randgalt commented on the issue: https://github.com/apache/zookeeper/pull/122 > Are you OK with us removing the deprecated methods when 3.5 becomes GA? No. I'd vote against that. Breaking changes should be in a 3.6 version. That was the genesis of this issue to begin with. Regardless of the intent of the ZK team, 3.5.x has been in production in a large number of companies for years now. To break a public API without increment the version will cause unnecessary problems. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 @Randgalt >> when are we going to be removing these deprecated methods, in trunk > Maybe when we get a stable release of 3.5? Are you OK with us removing the deprecated methods when 3.5 becomes GA? > Agree, for trunk the change would be just rename ZooKeeperAdmin::reconfig to ZooKeeperAdmin::reconfigure so it's consistent with branch-3.5 (with some documentation updates and tests update.). We need to either create a new issue or have a different pull request for 3.5 under this same issue. I'd rather do the latter just so that we wrap this up in one shot, but it depends on whether @Randgalt is willing to do it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 I think this patch is ready to land... @fpj What do you think? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 >> when are we going to be removing these deprecated methods, in trunk Maybe when we get a stable release of 3.5? >> it sounds like we don't need to bring back the old reconfig methods. Agree, for trunk the change would be just rename ZooKeeperAdmin::reconfig to ZooKeeperAdmin::reconfigure so it's consistent with branch-3.5 (with some documentation updates and tests update.). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user fpj commented on the issue: https://github.com/apache/zookeeper/pull/122 Just so that I understand, when are we going to be removing this methods, in trunk? I'm asking this for two reasons: 1. So that users know in which version these methods are going away 2. What changes we need to apply to trunk. In trunk, we would need to at least change in the admin class `reconfig()` to `reconfigure()`, but it sounds like we don't need to bring back the old `reconfig` methods. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #122: [ZOOKEEPER-2642] Resurrect the reconfig() methods that...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/122 Please also update the reconfig documentation about deprecating old API and about the new API in ZooKeeperAdmin: https://github.com/apache/zookeeper/blob/master/src/docs/src/documentation/content/xdocs/zookeeperReconfig.xml#L299 Other than this, the patch lgtm. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
