Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-2573: Modify Info.REVISION to adapt git repo #155

Closed
wants to merge 2 commits into from
Closed

ZOOKEEPER-2573: Modify Info.REVISION to adapt git repo #155

wants to merge 2 commits into from

Conversation

eribeiro
Copy link
Contributor

@eribeiro eribeiro commented Jan 24, 2017

@rakeshadr Hi, I have created this PR. The commit can be cherry picked on master too. In fact, I guess when you merge this then #137 will be automatically closed. I tested quickly and was able to cherry-pick this commit on branch-3.4 too, so you may want to give it a try. 😃

Copy link
Contributor

@rakeshadr rakeshadr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm able to merge this pull request to master and branch-3.5.

I just directly tried to apply this to branch-3.4 and got below minor conflicts. Let me try to cherry pick this to branch-3.4 while committing. If I get any conflicts then I will use #137 pull request for branch-3.4
error: patch failed: build.xml:312 error: build.xml: patch does not apply

rev = -1;
String rev = args[1];
if (rev == null || rev.trim().isEmpty()) {
rev = "-1";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Else block is missing. Could you add rev.trim() function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, really sorry about that. 😞 I squashed the commits from #137, but for whatever reason this latest change didn't make through. 🤔 Well, gonna change now. Thanks for pointing out!

@rakeshadr
Copy link
Contributor

Thanks @eribeiro.
+1 LGTM, I will merge this shortly.

@eribeiro
Copy link
Contributor Author

eribeiro commented Jan 25, 2017

Thanks for your patience and support @rakeshadr.

Indeed, I tried to git apply the patch: https://github.com/apache/zookeeper/pull/155.diff on branch-3.4 and it failed as you wrote before. 😞

Oddly enough, if I apply this patch to master or branch-3.5 (git apply 155.diff) then switched to branch-3.4, did a git cherry-pick <commit> then it worked. But I am suspicious if this was okay (I did a quick test here now).

asfgit pushed a commit that referenced this pull request Jan 25, 2017
rakeshadr Hi, I have created this PR. The commit can be cherry picked on master too. In fact, I guess when you merge this then #137 will be automatically closed. I tested quickly and was able to cherry-pick this commit on branch-3.4 too, so you may want to give it a try. 😃

Author: Edward Ribeiro <edward.ribeiro@gmail.com>
Author: Edward Ribeiro <eribeiro@users.noreply.github.com>

Reviewers: Mohammad Arshad <arshad@apache.org>, Michael Han <hanm@apache.org>

Closes #155 from eribeiro/ZOOKEEPER-2573-3.5
@asfgit asfgit closed this in 8771ffd Jan 25, 2017
asfgit pushed a commit that referenced this pull request Jan 25, 2017
rakeshadr Hi, I have created this PR. The commit can be cherry picked on master too. In fact, I guess when you merge this then #137 will be automatically closed. I tested quickly and was able to cherry-pick this commit on branch-3.4 too, so you may want to give it a try. 😃

Author: Edward Ribeiro <edward.ribeiro@gmail.com>
Author: Edward Ribeiro <eribeiro@users.noreply.github.com>

Reviewers: Mohammad Arshad <arshad@apache.org>, Michael Han <hanm@apache.org>

Closes #155 from eribeiro/ZOOKEEPER-2573-3.5

(cherry picked from commit 41da3c8)
Signed-off-by: Rakesh Radhakrishnan <rakeshr@apache.org>
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
rakeshadr Hi, I have created this PR. The commit can be cherry picked on master too. In fact, I guess when you merge this then apache#137 will be automatically closed. I tested quickly and was able to cherry-pick this commit on branch-3.4 too, so you may want to give it a try. 😃

Author: Edward Ribeiro <edward.ribeiro@gmail.com>
Author: Edward Ribeiro <eribeiro@users.noreply.github.com>

Reviewers: Mohammad Arshad <arshad@apache.org>, Michael Han <hanm@apache.org>

Closes apache#155 from eribeiro/ZOOKEEPER-2573-3.5

(cherry picked from commit 41da3c8)
Signed-off-by: Rakesh Radhakrishnan <rakeshr@apache.org>
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
rakeshadr Hi, I have created this PR. The commit can be cherry picked on master too. In fact, I guess when you merge this then apache#137 will be automatically closed. I tested quickly and was able to cherry-pick this commit on branch-3.4 too, so you may want to give it a try. 😃

Author: Edward Ribeiro <edward.ribeiro@gmail.com>
Author: Edward Ribeiro <eribeiro@users.noreply.github.com>

Reviewers: Mohammad Arshad <arshad@apache.org>, Michael Han <hanm@apache.org>

Closes apache#155 from eribeiro/ZOOKEEPER-2573-3.5

(cherry picked from commit 41da3c8)
Signed-off-by: Rakesh Radhakrishnan <rakeshr@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants