integrations: Remove !avatar from git and game handler. #649
Conversation
The issue linked to this commit suggest suggests to replace the avatar with the username only, I just needed to remove !avatar as the code already shows username. Fixes: zulip#632
ahmedabuamra
changed the title
zulip/integrations/git/post-receive
Outdated
| log_cmd = ["git", "log", "--reverse", | ||
| "--pretty=%aE %H %s", "%s..%s" % (oldrev, newrev)] | ||
| "--pretty=-[AUTHOR]=%aN-[COMMIT]=%H-[SUBJECT]=%s", "%s..%s" % (oldrev, newrev)] |
There was a problem hiding this comment.
It’d be more robust and simpler to separate with newlines (--pretty=%aN%n%H%n%h%n%s). This can be parsed without any fanfare:
it = subprocess.check_output(log_cmd, universal_newlines=True).splitlines()
for author_name in it:
commit_hash = next(it)
commit_short_hash = next(it)
subject = next(it)
commits += git.COMMIT_ROW_TEMPLATE.format(
author_name=author_name,
commit_short_hash=commit_short_hash,
subject=subject,
commit_url="{}/commit/{}".format(remote_repo_url, commit_hash),
)
zulip/integrations/git/post-receive
Outdated
| author, commit_sha, subject = git.extract_commit_data(ln) | ||
| commits += git.COMMIT_ROW_TEMPLATE.format( | ||
| user_name=author, | ||
| commit_short_sha=commit_sha[:7], |
There was a problem hiding this comment.
Let Git compute the abbreviation length with %h instead of using a fixed length. In large repositories, it automatically increases the abbreviation length to avoid collisions.
There was a problem hiding this comment.
The webhook has less information since it doesn’t have the repository available to it locally.
zulip/integrations/git/post-receive
Outdated
| commits += '!avatar(%s) %s\n' % (author_email, subject) | ||
| author, commit_sha, subject = git.extract_commit_data(ln) | ||
| commits += git.COMMIT_ROW_TEMPLATE.format( | ||
| user_name=author, |
There was a problem hiding this comment.
This is an author name, not a username. There’s no reason to rename all the variables passed to the template.
zulip/integrations/git/post-receive
Outdated
| oldrev == '0000000000000000000000000000000000000000' | ||
| or newrev == '0000000000000000000000000000000000000000' | ||
| ): | ||
| if (oldrev == git.EMPTY_SHA or newrev == git.EMPTY_SHA): |
|
@andersk thank you for your comments! I agree with all of them and will update you once resolved. |
…ead. Getting the logs run in cmd as in function `git_commit_range(oldrev, newrev)` so I had to prefix author name with -[AUTHOR], commit with -[COMMIT] and subject/title with -[SUBJECT] to be able to extract these info correctly. I also tried to stick with the format proposed in this issue zulip/zulip#3968 except for showing the author's name before each commit and without summary. Fixes: zulip#632
ahmedabuamra
force-pushed
the
abuamra-avatar-removal
branch
from
d40a41a to
667c4ac
Compare
| commit_short_hash=commit_short_hash, | ||
| subject=subject, | ||
| commit_url="{}/commit/{}".format(remote_repo_url, commit_hash) | ||
| ) |
There was a problem hiding this comment.
For the formatting, I think it'd be better to actually match the format in https://chat.zulip.org/#narrow/stream/8-commits/topic/zulip.20.2F.20master/near/1121032. Is there a reason you went with a different style?
|
This looks pretty good; I merged the first commit as 9ce2ea5 (after updating the commit message to say |
|
@ahmedabuamra have you had a chance to resolve the feedback on this? |
|
@timabbott I am sorry for not working on it till now. I have had exams from the beginning of this month to yesterday. |
|
Heads up @ahmedabuamra, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Fixes: #632
Message content example (on my forked repo.):
Please note this sample is generated with input:
5f81eb3a7071fa0dfc20335d01ad889edca27f4e e995e52896a9fa93678eedb777d6d63927913bb5 refs/heads/masterwhich formatting output of:
git log --reverse --pretty=%aN%n%H%n%h%n%s 5f81eb3a7071fa0dfc20335d01ad889edca27f4e..e995e52896a9fa93678eedb777d6d63927913bb5