You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2024/09/13 13:58:02 UTC
[PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
pan3793 opened a new pull request, #4831:
URL: https://github.com/apache/zeppelin/pull/4831
### What is this PR for?
Zeppelin has `dev/merge_zeppelin_pr.py`, I would recommend committers to use this script over the GitHub button to merge PR, which has some benefits:
1. Simplify the backport process
the tools will ask you to backport the commit to lower maintained branches after you merge a PR to master, if there are no conflicts, all things you need to do are just type a "branch name" that you want to backport.
2. Automatically update JIRA information
the script uses the python jira client to update JIRA ticket, for example, fill in the fixed versions, which is important to users to know the features/bug fixes applied versions.
3. Better PR title, body, and committer info
Zeppelin
<img width="1001" alt="image" src="https://github.com/user-attachments/assets/b709f5f1-0194-4472-885a-8336db3bc9d4">
Spark
<img width="992" alt="image" src="https://github.com/user-attachments/assets/c16d9469-15bd-4158-b745-c6b21df7574f">
### What type of PR is it?
Improvement
### Todos
* [ ] - revert https://github.com/apache/zeppelin/pull/4452
* [ ] - verify this script by merging at least 3 PRs
### What is the Jira issue?
ZEPPELIN-6089
### How should this be tested?
Manually test.
```
(zeppelin) ➜ apache-zeppelin git:(ZEPPELIN-6089) dev/merge_zeppelin_pr.py
git rev-parse --abbrev-ref HEAD
Which pull request would you like to merge? (e.g. 34): 4807
=== JIRA ZEPPELIN-6073 ===
Summary Canonicalize binary distribution tarball name
Assignee None
Status Open
Url https://issues.apache.org/jira/browse/ZEPPELIN-6073
Affected []
=== Pull Request #4807 ===
title [ZEPPELIN-6073][BUILD] Canonicalize binary distribution tarball name
source pan3793/ZEPPELIN-6073
target master
url https://api.github.com/repos/apache/zeppelin/pulls/4807
Proceed with merging pull request #4807? (y/N): y
git fetch apache pull/4807/head:PR_TOOL_MERGE_PR_4807
From github.com:apache/zeppelin
* [new ref] refs/pull/4807/head -> PR_TOOL_MERGE_PR_4807
git fetch apache master:PR_TOOL_MERGE_PR_4807_MASTER
From github.com:apache/zeppelin
* [new branch] master -> PR_TOOL_MERGE_PR_4807_MASTER
git checkout PR_TOOL_MERGE_PR_4807_MASTER
Switched to branch 'PR_TOOL_MERGE_PR_4807_MASTER'
['git', 'merge', 'PR_TOOL_MERGE_PR_4807', '--squash']
Automatic merge went well; stopped before committing as requested
['git', 'log', 'HEAD..PR_TOOL_MERGE_PR_4807', '--pretty=format:%an <%ae>', '--reverse']
Enter primary author in the format of "name <email>" [Cheng Pan <ch...@apache.org>]:
git config --get user.name
git config --get user.email
['git', 'commit', '--author="Cheng Pan <ch...@apache.org>"', '-m', '[ZEPPELIN-6073][BUILD] Canonicalize binary distribution tarball name', '-m', '### What is this PR for?\r\n\r\nThis PR aims to canonicalize the binary distribution tarball name assembled by Maven.\r\n\r\nCurrently, the `./mvnw clean package -Pbuild-distr -DskipTests` produces `zeppelin-0.12.0-SNAPSHOT.tar.gz`, while the final release tarballs are `zeppelin-0.11.1-bin-all.tgz` and `zeppelin-0.11.1-bin-netinst.tgz`.\r\n\r\nAfter this change, `./mvnw clean package -Pbuild-distr -DskipTests` produces `zeppelin-0.12.0-SNAPSHOT-bin.tgz`.\r\n\r\n### What type of PR is it?\r\n\r\nImprovement\r\n\r\n### What is the Jira issue?\r\n\r\nZEPPELIN-6073\r\n\r\n### How should this be tested?\r\n\r\nPass existing GHA and manually run maven command to package the binary tarball.\r\n\r\n```\r\n$ ./mvnw clean package -Pbuild-distr -DskipTests\r\n```\r\n\r\n```\r\n...\r\n[INFO] -------------< org.apache.zeppelin:zeppelin-distributio
n >--------------\r\n[INFO] Building Zeppelin: Packaging distribution 0.12.0-SNAPSHOT [64/64]\r\n[INFO] --------------------------------[ pom ]---------------------------------\r\n...\r\n[INFO] --- maven-assembly-plugin:3.7.1:single (make-assembly) zeppelin-distribution ---\r\n[INFO] Reading assembly descriptor: src/assemble/distribution.xml\r\n[INFO] Copying files to /Users/chengpan/Projects/apache-zeppelin/zeppelin-distribution/target/zeppelin-0.12.0-SNAPSHOT-bin\r\n[INFO] 838 files copied to /Users/chengpan/Projects/apache-zeppelin/zeppelin-distribution/target/zeppelin-0.12.0-SNAPSHOT-bin\r\n[INFO] Building tar: /Users/chengpan/Projects/apache-zeppelin/zeppelin-distribution/target/zeppelin-0.12.0-SNAPSHOT-bin.tgz\r\n...\r\n```\r\n\r\n### Screenshots (if appropriate)\r\n\r\n### Questions:\r\n* Does the license files need to update? No\r\n* Is there breaking changes for older versions? No\r\n* Does this needs documentation? No\r\n', '-m', 'Closes #4807 from pan3793/ZEPPELIN
-6073.', '-m', 'Authored-by: Cheng Pan <ch...@apache.org>\nSigned-off-by: Cheng Pan <ch...@apache.org>']
husky > pre-commit (node v18.20.4)
No staged files match any of provided globs.
Merge complete (local ref PR_TOOL_MERGE_PR_4807_MASTER). Push to apache? (y/N): y
git push apache PR_TOOL_MERGE_PR_4807_MASTER:master
Enumerating objects: 39, done.
Counting objects: 100% (39/39), done.
Delta compression using up to 10 threads
Compressing objects: 100% (19/19), done.
Writing objects: 100% (20/20), 3.04 KiB | 3.04 MiB/s, done.
Total 20 (delta 14), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (14/14), completed with 14 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote:
remote: - Changes must be made through a pull request.
To github.com:apache/zeppelin.git
! [remote rejected] PR_TOOL_MERGE_PR_4807_MASTER -> master (protected branch hook declined)
error: failed to push some refs to 'github.com:apache/zeppelin.git'
...
```
### Screenshots (if appropriate)
### Questions:
* Does the license files need to update? No.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2360205749
@Reamer your concerns are also addressed, now, the merged PR status is "Merged"
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359937511
> this involves another drawback https://github.com/orgs/community/discussions/32934
But it preserves the author. Could we allow 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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2354276423
IIRC, this script closes PRs instead of merging them. Can we use github API to merge the PR? It's the reason why I prefer to use github's merge button.
> to enable this capability, I need to revert https://github.com/apache/zeppelin/pull/4452
If the script works properly, I don't care to revert 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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359869200
@jongyoul GitHub Actions can not solve backport and update JIRA information issues.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#discussion_r1758935590
##########
dev/merge_zeppelin_pr.py:
##########
@@ -267,95 +350,366 @@ def resolve_jira_issue(merge_branches, comment, default_jira_id=""):
default_fix_versions = list(filter(lambda x: x != v, default_fix_versions))
default_fix_versions = ",".join(default_fix_versions)
- fix_versions = input("Enter comma-separated fix version(s) [%s]: " % default_fix_versions)
- if fix_versions == "":
- fix_versions = default_fix_versions
- fix_versions = fix_versions.replace(" ", "").split(",")
+ available_versions = set(list(map(lambda v: v.name, versions)))
+ while True:
+ try:
+ fix_versions = bold_input(
+ "Enter comma-separated fix version(s) [%s]: " % default_fix_versions
+ )
+ if fix_versions == "":
+ fix_versions = default_fix_versions
+ fix_versions = fix_versions.replace(" ", "").split(",")
+ if set(fix_versions).issubset(available_versions):
+ break
+ else:
+ print(
+ "Specified version(s) [%s] not found in the available versions, try "
+ "again (or leave blank and fix manually)." % (", ".join(fix_versions))
+ )
+ except KeyboardInterrupt:
+ raise
+ except BaseException:
+ traceback.print_exc()
+ print("Error setting fix version(s), try again (or leave blank and fix manually)")
def get_version_json(version_str):
return list(filter(lambda v: v.name == version_str, versions))[0].raw
jira_fix_versions = list(map(lambda v: get_version_json(v), fix_versions))
- resolve = list(filter(lambda a: a['name'] == "Resolve Issue", asf_jira.transitions(jira_id)))[0]
+ resolve = list(filter(lambda a: a["name"] == "Resolve Issue", asf_jira.transitions(issue.key)))[
+ 0
+ ]
+ resolution = list(filter(lambda r: r.raw["name"] == "Fixed", asf_jira.resolutions()))[0]
asf_jira.transition_issue(
- jira_id, resolve["id"], fixVersions=jira_fix_versions, comment=comment)
+ issue.key,
+ resolve["id"],
+ fixVersions=jira_fix_versions,
+ comment=comment,
+ resolution={"id": resolution.raw["id"]},
+ )
- print("Succesfully resolved %s with fixVersions=%s!" % (jira_id, fix_versions))
+ try:
+ print_jira_issue_summary(asf_jira.issue(issue.key))
+ except Exception:
+ print("Unable to fetch JIRA issue %s after resolving" % issue.key)
+ print("Successfully resolved %s with fixVersions=%s!" % (issue.key, fix_versions))
+
+
+def choose_jira_assignee(issue):
+ """
+ Prompt the user to choose who to assign the issue to in jira, given a list of candidates,
+ including the original reporter and all commentators
+ """
+ while True:
+ try:
+ reporter = issue.fields.reporter
+ commentators = list(map(lambda x: x.author, issue.fields.comment.comments))
+ candidates = set(commentators)
+ candidates.add(reporter)
+ candidates = list(candidates)
+ print("JIRA is unassigned, choose assignee")
+ for idx, author in enumerate(candidates):
+ if author.key == "apachezeppelin":
+ continue
+ annotations = ["Reporter"] if author == reporter else []
+ if author in commentators:
+ annotations.append("Commentator")
+ print("[%d] %s (%s)" % (idx, author.displayName, ",".join(annotations)))
+ raw_assignee = bold_input(
+ "Enter number of user, or userid, to assign to (blank to leave unassigned):"
+ )
+ if raw_assignee == "":
+ return None
+ else:
+ try:
+ id = int(raw_assignee)
+ assignee = candidates[id]
+ except BaseException:
+ # assume it's a user id, and try to assign (might fail, we just prompt again)
+ assignee = asf_jira.user(raw_assignee)
+ try:
+ assign_issue(issue.key, assignee.name)
+ except Exception as e:
+ if (
+ e.__class__.__name__ == "JIRAError"
+ and ("'%s' cannot be assigned" % assignee.name)
+ in getattr(e, "response").text
+ ):
+ continue_maybe(
+ "User '%s' cannot be assigned, add to contributors role and try again?"
+ % assignee.name
+ )
+ grant_contributor_role(assignee.name)
+ assign_issue(issue.key, assignee.name)
+ else:
+ raise e
+ return assignee
+ except KeyboardInterrupt:
+ raise
+ except BaseException:
+ traceback.print_exc()
+ print("Error assigning JIRA, try again (or leave blank and fix manually)")
+
+
+def grant_contributor_role(user: str):
+ role = asf_jira.project_role("ZEPPELIN", 10010)
Review Comment:
not sure if this is correct, I don't have the Zeppelin project JIRA admin permission
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359919464
> keep proper histories for Github
If you are speaking the PR status, yes, the status merged by the script is "Closed" instead of "Merged", this can not be fixed due to the GitHub API limitation. I don't see contributors confused with that in the other projects that use a similar script for merging PR if we leave proper words at the PR end like "Merged to master/3.5".
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2365277034
Thanks, merged to master for 0.12
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#discussion_r1758935590
##########
dev/merge_zeppelin_pr.py:
##########
@@ -267,95 +350,366 @@ def resolve_jira_issue(merge_branches, comment, default_jira_id=""):
default_fix_versions = list(filter(lambda x: x != v, default_fix_versions))
default_fix_versions = ",".join(default_fix_versions)
- fix_versions = input("Enter comma-separated fix version(s) [%s]: " % default_fix_versions)
- if fix_versions == "":
- fix_versions = default_fix_versions
- fix_versions = fix_versions.replace(" ", "").split(",")
+ available_versions = set(list(map(lambda v: v.name, versions)))
+ while True:
+ try:
+ fix_versions = bold_input(
+ "Enter comma-separated fix version(s) [%s]: " % default_fix_versions
+ )
+ if fix_versions == "":
+ fix_versions = default_fix_versions
+ fix_versions = fix_versions.replace(" ", "").split(",")
+ if set(fix_versions).issubset(available_versions):
+ break
+ else:
+ print(
+ "Specified version(s) [%s] not found in the available versions, try "
+ "again (or leave blank and fix manually)." % (", ".join(fix_versions))
+ )
+ except KeyboardInterrupt:
+ raise
+ except BaseException:
+ traceback.print_exc()
+ print("Error setting fix version(s), try again (or leave blank and fix manually)")
def get_version_json(version_str):
return list(filter(lambda v: v.name == version_str, versions))[0].raw
jira_fix_versions = list(map(lambda v: get_version_json(v), fix_versions))
- resolve = list(filter(lambda a: a['name'] == "Resolve Issue", asf_jira.transitions(jira_id)))[0]
+ resolve = list(filter(lambda a: a["name"] == "Resolve Issue", asf_jira.transitions(issue.key)))[
+ 0
+ ]
+ resolution = list(filter(lambda r: r.raw["name"] == "Fixed", asf_jira.resolutions()))[0]
asf_jira.transition_issue(
- jira_id, resolve["id"], fixVersions=jira_fix_versions, comment=comment)
+ issue.key,
+ resolve["id"],
+ fixVersions=jira_fix_versions,
+ comment=comment,
+ resolution={"id": resolution.raw["id"]},
+ )
- print("Succesfully resolved %s with fixVersions=%s!" % (jira_id, fix_versions))
+ try:
+ print_jira_issue_summary(asf_jira.issue(issue.key))
+ except Exception:
+ print("Unable to fetch JIRA issue %s after resolving" % issue.key)
+ print("Successfully resolved %s with fixVersions=%s!" % (issue.key, fix_versions))
+
+
+def choose_jira_assignee(issue):
+ """
+ Prompt the user to choose who to assign the issue to in jira, given a list of candidates,
+ including the original reporter and all commentators
+ """
+ while True:
+ try:
+ reporter = issue.fields.reporter
+ commentators = list(map(lambda x: x.author, issue.fields.comment.comments))
+ candidates = set(commentators)
+ candidates.add(reporter)
+ candidates = list(candidates)
+ print("JIRA is unassigned, choose assignee")
+ for idx, author in enumerate(candidates):
+ if author.key == "apachezeppelin":
+ continue
+ annotations = ["Reporter"] if author == reporter else []
+ if author in commentators:
+ annotations.append("Commentator")
+ print("[%d] %s (%s)" % (idx, author.displayName, ",".join(annotations)))
+ raw_assignee = bold_input(
+ "Enter number of user, or userid, to assign to (blank to leave unassigned):"
+ )
+ if raw_assignee == "":
+ return None
+ else:
+ try:
+ id = int(raw_assignee)
+ assignee = candidates[id]
+ except BaseException:
+ # assume it's a user id, and try to assign (might fail, we just prompt again)
+ assignee = asf_jira.user(raw_assignee)
+ try:
+ assign_issue(issue.key, assignee.name)
+ except Exception as e:
+ if (
+ e.__class__.__name__ == "JIRAError"
+ and ("'%s' cannot be assigned" % assignee.name)
+ in getattr(e, "response").text
+ ):
+ continue_maybe(
+ "User '%s' cannot be assigned, add to contributors role and try again?"
+ % assignee.name
+ )
+ grant_contributor_role(assignee.name)
+ assign_issue(issue.key, assignee.name)
+ else:
+ raise e
+ return assignee
+ except KeyboardInterrupt:
+ raise
+ except BaseException:
+ traceback.print_exc()
+ print("Error assigning JIRA, try again (or leave blank and fix manually)")
+
+
+def grant_contributor_role(user: str):
+ role = asf_jira.project_role("ZEPPELIN", 10010)
Review Comment:
not sure if this is correct, ~~I don't have the Zeppelin project JIRA admin permission~~
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359887879
@jongyoul the script is interactive, it asks the committer to fill in the backport branch and do a local cherry-pick before pushing to repo, if there are some conflicts, it allows the committer to resove it manually and continue the process, in some cases, the cherry-picked code has no line-level conflicts but breaks the compile, committers still have a chance to do a local compile before pushing the code. For updating JIRA, if we use the GitHub Actions, how to tell that the fixed version? by a comment in special words? it's another burden.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359924657
> this can not be fixed due to the GitHub API limitation.
We can fix it by squashing it locally in the script pushing it, and calling Github API to merge the PR instead of committing and pushing the PR directly to master.
> I don't see contributors confused with that
When I was a contributor, I was confused really because I didn't know why my PR was closed instead of merging even though it exists in the master branch. That's why I insisted on using the Giuthub merge button and tried to work others manually like closing JIRA with the proper fix version and cherry-picking some commits by myself.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359953218
Though this violates the git design, given the committer's name is recorded in the commit message body, I'm OK with this approach, let me try this way.
```
Authored-by: Author Name <au...@mail.address>
Signed-off-by: Committer Name <co...@mail.address>
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359942536
concerns are
> it is the committing person who is responsible for the decision to merge. So it is them who should be recorded in Commit[ter], not GitHub.
I tend to agree with that.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2360178806
@jongyoul I have updated the script to use the GitHub open API you suggested to merge the PR, and have merged 4 PRs 876d1dc7fd460fffbe855873bc5a65865bcd0f78 81783ddc4c8d32ec8e606fa13a966227b747d2ae ad79848a9035d1e5ea596632dac523b757aaaf8a 35e12991262566a4ac7a3fce11ab8dbd08d05cde to test the cases:
- fail-fast if the PR has no approval
- commit message contains PR description and "Signed-off-by" info
- auto backport
- auto update JIRA ticket
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359912902
Yes, right. we need to make another function to handle this way. By the way, I agree with Reamer. we need to keep proper histories for Github because most interactions with contributors happen in Github. Merging issues are not dependent on contributors but contributions are related to contributors directly, so I feel like we need to prioritize contributors' experiences.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359878678
> @jongyoul GitHub Actions can not solve backport and update JIRA information issues.
I mean using the logic implemented in our current script.
For cherry-pick, we can make a ticket like
```
name: Cherry-pick on Comment
on:
issue_comment:
types: [created]
jobs:
cherry-pick-on-comment:
runs-on: ubuntu-latest
if: contains(github.event.comment.body, '/cherry-pick')
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- name: Parse cherry-pick branch from comment
id: parse_branch
run: |
COMMENT_BODY="${{ github.event.comment.body }}"
if [[ "$COMMENT_BODY" =~ /cherry-pick\ ([^\ ]+) ]]; then
echo "::set-output name=branch::${BASH_REMATCH[1]}"
else
echo "Cherry-pick command not found"
exit 1
fi
- name: Set up Git
run: |
git config user.name "Your Name"
git config user.email "your-email@example.com"
- name: Fetch and cherry-pick the commit
run: |
TARGET_BRANCH=${{ steps.parse_branch.outputs.branch }}
git fetch origin $TARGET_BRANCH
git checkout $TARGET_BRANCH
git cherry-pick ${{ github.event.issue.pull_request.merge_commit_sha }}
git push origin $TARGET_BRANCH
```
It will work /cherry-pick branch-0.11
For JIRA Update,
```
name: Close JIRA Ticket on Merge
on:
pull_request:
types:
- closed
jobs:
close-jira-ticket:
runs-on: ubuntu-latest
if: github.event.pull_request.merged == true
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- name: Extract ticket number from PR title
id: extract_ticket
run: |
PR_TITLE="${{ github.event.pull_request.title }}"
if [[ "$PR_TITLE" =~ ([A-Z]+-[0-9]+) ]]; then
echo "::set-output name=ticket::${BASH_REMATCH[1]}"
else
echo "No JIRA ticket number found in the title"
exit 1
fi
- name: Update JIRA ticket status to Done
run: |
TICKET_NUMBER=${{ steps.extract_ticket.outputs.ticket }}
curl -X PUT -H "Authorization: Basic ${{ secrets.JIRA_AUTH }}" \
-H "Content-Type: application/json" \
--data '{"transition": {"id": "31"}}' \
"https://your-jira-domain.atlassian.net/rest/api/3/issue/$TICKET_NUMBER/transitions"
```
It will update JIRA ticket as well.
FYI, this code is generated by GPT and we should modify something. I would just like to give you an idea.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2349034545
to enable this capability, I need to revert https://github.com/apache/zeppelin/pull/4452
@jongyoul @Reamer @zjffdu what do you think about 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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2354658417
It is more important to me that we have a reasonable GitHub history than the history on the mirrored Apache Git instance.
The old merge script marked the pull request as closed, which doesn't reflect the history.
In my opinion, most developers are working on the Zeppelin code via GitHub. Where did you get the screenshots?
If we get both, that's fine with me, but I don't think the Squash&Merge feature should be disabled for that.
Working with a merge script is fine with me. As you wrote, this can simplify many things.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359878191
Additionally, I think it's hard to write a rule to evaluate whether the PR is ready for merge.
Even if the PR receives sufficient approval, it still has a chance of having unaddressed comments or concerns from other reviewers. Committing code to the codebase is serious, and I think we should judge the check-in condition manually(either by clicking a button, or by using a script).
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359884293
I also don't want to merge some commits automatically. After reviewing by reviewers, only the commits will be squashed and pushed forcibly into the same PR branch. (We should check if it could work with contributors' branches)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359932497
> We can fix it by squashing it locally in the script, pushing it to the PR, and calling Github API to merge the PR instead of committing and pushing the PR directly to the master.
this involves another drawback https://github.com/orgs/community/discussions/32934
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 merged PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2356448238
> ... this script closes PRs instead of merging them.
> ... merge script marked the pull request as closed, which doesn't reflect the history.
Yes, this is a drawback. If you look at Spark's PRs, the committer who merged the PR will clarify that by commenting something like "Merged to master/3.5".
> Where did you get the screenshots?
A git GUI client, https://fork.dev/
> ... I don't think the Squash&Merge feature should be disabled for that.
Okay, and I'm not against using this approach, as long as the committer does not forget to do other things manually.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359927125
I found an API for merging PR with proper titles and messages. Can we investigate it?
- https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#merge-a-pull-request
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359952870
Agreed. That's the reason we do the approve button. We can guarantee it by checking the PR's approval. If some PRs weren't approved and we found it, we can revert 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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#issuecomment-2359844127
I have a new idea. How about using github action more? E.g. we can trigger it by approving it, then change the commit message with squash and amend the commit. WDYT?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP][ZEPPELIN-6089][INFRA] Improve the merge PR script [zeppelin]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4831:
URL: https://github.com/apache/zeppelin/pull/4831#discussion_r1758933311
##########
dev/merge_zeppelin_pr.py:
##########
@@ -45,13 +43,24 @@
# Location of your Zeppelin git development area
ZEPPELIN_HOME = os.environ.get("ZEPPELIN_HOME", os.getcwd())
# Remote name which points to the Github site
-PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME", "apache-github")
+PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME", "apache")
# Remote name which points to Apache git
PUSH_REMOTE_NAME = os.environ.get("PUSH_REMOTE_NAME", "apache")
# ASF JIRA username
-JIRA_USERNAME = os.environ.get("JIRA_USERNAME", "moon")
+JIRA_USERNAME = os.environ.get("JIRA_USERNAME", "")
# ASF JIRA password
-JIRA_PASSWORD = os.environ.get("JIRA_PASSWORD", "00000")
+JIRA_PASSWORD = os.environ.get("JIRA_PASSWORD", "")
+# ASF JIRA access token
+# If it is configured, username and password are dismissed
+# Go to https://issues.apache.org/jira/secure/ViewProfile.jspa -> Personal Access Tokens for
+# your own token management.
+JIRA_ACCESS_TOKEN = os.environ.get("JIRA_ACCESS_TOKEN")
+# OAuth key used for issuing requests against the GitHub API. If this is not defined, then requests
+# will be unauthenticated. You should only need to configure this if you find yourself regularly
+# exceeding your IP's unauthenticated request rate limit. You can create an OAuth key at
+# https://github.com/settings/tokens. This script only requires the "public_repo" scope.
+GITHUB_OAUTH_KEY = os.environ.get("GITHUB_OAUTH_KEY")
Review Comment:
both JIRA and GITHUB encourage using TOKEN instead of PASSWORD for security purposes.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org