You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mumrah (via GitHub)" <gi...@apache.org> on 2025/03/19 14:38:59 UTC
[PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
mumrah opened a new pull request, #19242:
URL: https://github.com/apache/kafka/pull/19242
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2740272180
Did we check that GitHub doesn't do this automatically?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #19242:
URL: https://github.com/apache/kafka/pull/19242
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2737080357
To test this out, checkout the PR and run the following:
```
PR_NUMBER=19071 python .github/scripts/pr-format.py
```
⚠️ Do not run with GITHUB_ACTIONS set, or it will actually update the PR. ⚠️
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2752805699
I started a DISCUSS thread about this cc @ijuma @dajac @chia7712
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2750756701
@mumrah I have noticed that it also breaks the `Reviewers` line. This will make listing commits reviewed by someone harder, doesn't it? I wonder whether we should also switch to using `Reviewed-by ...` tag similar to the `Co-authored-by` tag. We would have one reviewer per line with something like this.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19242:
URL: https://github.com/apache/kafka/pull/19242#discussion_r2007419408
##########
.github/scripts/pr-format.py:
##########
@@ -132,6 +152,32 @@ def check(positive_assertion, ok_msg, err_msg):
check("Delete this text and replace" not in body, "PR template text not present", "PR template text should be removed")
check("Committer Checklist" not in body, "PR template text not present", "Old PR template text should be removed")
+ paragraph_iter = split_paragraphs(body)
+ new_paragraphs = []
+ for p, markdown in paragraph_iter:
+ if markdown:
+ # If a paragraph looks like it has markdown in it, wrap each line separately.
+ new_lines = []
+ for line in p:
+ new_lines.append(textwrap.fill(line, width=72, break_long_words=False, break_on_hyphens=False, replace_whitespace=False))
+ rewrapped_p = "\n".join(new_lines)
+ else:
+ rewrapped_p = textwrap.fill("".join(p), width=72, break_long_words=False, break_on_hyphens=False, replace_whitespace=True)
+ new_paragraphs.append(rewrapped_p + "\n")
+ body = "\n".join(new_paragraphs)
+
+ if get_env("GITHUB_ACTIONS"):
+ with tempfile.NamedTemporaryFile() as fp:
+ fp.write(body.encode())
+ fp.flush()
+ cmd = f"gh pr edit {pr_number} --body-file {fp.name}"
Review Comment:
Given that we intend to utilize GitHub Actions for pull request description reformatting, would it be feasible to extend its functionality to automatically populate the reviewers list?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2752507990
@dajac yes that's right. We can see that GH happily breaks existing trailers with it's auto-reformatting https://github.com/apache/kafka-merge-queue-sandbox/commit/27183691c299612a6f3d50a858def019fad3cbbe
I think it's a good idea to use one `Reviewed-By` per reviewer.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2743458057
One advantage of leaving it to github is that the PR message itself isn't limited to the shorter line length. Pretty sad that GitHub doesn't do a good enough job. :(
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2743749389
@ijuma I don't think GH will truncate the commit subject, but I'm not sure -- I'll try it out on the sandbox repo.
It is _really_ annoying that we can't tweak the commit message with merge queues. Lots of people have complained about it, so hopefully it will get improved.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2743356688
> btw, do you plan to set PR Linter as blocker?
Eventually, yes. We'll want it to be a required check before we switch on the merge queue.
> would it be feasible to extend its functionality to automatically populate the reviewers list?
Yes, we could. There was some previous discussion around where to pull this data from (website vs a file in git). I had a prototype at some point that used a git-config type of file to store committer name, username, and email.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19013 Reformat PR body to 72 characters [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #19242:
URL: https://github.com/apache/kafka/pull/19242#issuecomment-2741300567
@ijuma GitHub _does_ reformat the PR body when turning it into a commit message. The reason I'd like to preemptively reformat PRs is so we don't have surprises with our formatting.
GitHub's text wrap is pretty naive and doesn't always do a great job. If we reformat the PR bodies ourselves, we can do it better and get a preview of what it will look like in the commit log.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org