Page MenuHomePhorge
Differential D25421

Audit Feed: less verbose when the author is the committer
ClosedPublic
Actions

Authored by valerio.bozzolan on Aug 28 2023, 12:09.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 2, 17:28
Unknown Object (File)
Wed, May 1, 17:01
Unknown Object (File)
Wed, May 1, 08:51
Unknown Object (File)
Wed, May 1, 08:51
Unknown Object (File)
Wed, May 1, 08:51
Unknown Object (File)
Wed, May 1, 08:51
Unknown Object (File)
Wed, May 1, 08:51
Unknown Object (File)
Wed, May 1, 08:51
View All Files
Subscribers
Cigaryno
Matthew
speck
tobiaswiese

Details

Reviewers
speck
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15528: Diffusion commit Feed: avoid to show the author twice (Authored by xxx)
Commits
rP775d141fe6bf: Audit Feed: less verbose when the author is the committer
Summary

If the author and the committer are the same person, do not show them twice.

From:

UsernameFoo committed <commit hash>: <commit msg> (authored by UsernameFoo).

To:

UsernameFoo committed <commit hash>: <commit msg>

This only affects the feed.

BeforeAfter
Committer Feed before D25421.png (836×1 px, 152 KB)
Committer Feed after D25421.png (836×1 px, 140 KB)

Closes T15528

Test Plan

Do some mixed commits and visit /feed/query/all/. No nuclear implosions.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

valerio.bozzolan created this revision.Aug 28 2023, 12:09
Owners added a reviewer: O1: Blessed Committers.Aug 28 2023, 12:09
Herald added subscribers: Cigaryno, Matthew, tobiaswiese. · View Herald TranscriptAug 28 2023, 12:09
Harbormaster completed remote builds in B809: Diff 1336.Aug 28 2023, 12:09
valerio.bozzolan requested review of this revision.Aug 28 2023, 12:09
valerio.bozzolan updated this revision to Diff 1337.Aug 28 2023, 12:12

fix corner case of NULL phid

Harbormaster completed remote builds in B810: Diff 1337.Aug 28 2023, 12:12
valerio.bozzolan edited the summary of this revision. (Show Details)Aug 28 2023, 12:28
valerio.bozzolan updated this revision to Diff 1338.Aug 28 2023, 12:30

consider more cases

Harbormaster completed remote builds in B811: Diff 1338.Aug 28 2023, 12:30
valerio.bozzolan retitled this revision from Audit: do not show same author twice in Feed to Audit Feed: less verbose when the author is the committer.Aug 28 2023, 13:37
valerio.bozzolan added inline comments.Sep 6 2023, 11:09
src/applications/audit/storage/PhabricatorAuditTransaction.php
341–351

✅ This is the supposed non-empty check for a PHID. See line 323.

valerio.bozzolan added a comment.Nov 3 2023, 21:43

Basically it works

speck accepted this revision.Nov 4 2023, 15:07
speck subscribed.

Basically it works

😂

Great QoL change, thank you! I just had one thing to consider but leave it you to you to address or not before landing.

src/applications/audit/storage/PhabricatorAuditTransaction.php
341–351

What do you think of merging this logic above with the existing checks on author and committer?

This revision is now accepted and ready to land.Nov 4 2023, 15:07
valerio.bozzolan added inline comments.Nov 7 2023, 09:08
src/applications/audit/storage/PhabricatorAuditTransaction.php
341–351

Thanks speck. I tried for some minutes but failing.

Any idea?

valerio.bozzolan added inline comments.Nov 9 2023, 20:08
src/applications/audit/storage/PhabricatorAuditTransaction.php
341–351

Tried again refactoring, without success 🤔 Uhm. Need some sleep and/or help.

speck added inline comments.Nov 10 2023, 04:04
src/applications/audit/storage/PhabricatorAuditTransaction.php
322–352

This is what I was thinking but after taking further look I think I prefer the version you have.

valerio.bozzolan updated this revision to Diff 1497.Nov 13 2023, 10:26

Small improvement thanks to review tip

Harbormaster completed remote builds in B921: Diff 1497.Nov 13 2023, 10:26
valerio.bozzolan added a comment.Nov 13 2023, 10:28

(Hide all inline comments or this is unreadable)

Thanks for the tip. Maybe better now?

valerio.bozzolan marked an inline comment as done.Nov 14 2023, 13:23

(This was already approved but I wait for another "Yeah land" since I changed a thing)

speck accepted this revision.Nov 14 2023, 19:45

Minor, just suggestion

src/applications/audit/storage/PhabricatorAuditTransaction.php
343

This reads a little strange now. I think this check is fine to remain as “if author”

valerio.bozzolan updated this revision to Diff 1519.Nov 15 2023, 08:41

avoid micro-optimizations that make this unreadable

Harbormaster completed remote builds in B941: Diff 1519.Nov 15 2023, 08:41
speck accepted this revision.Nov 15 2023, 12:49
Closed by commit rP775d141fe6bf: Audit Feed: less verbose when the author is the committer. · Explain WhyNov 15 2023, 12:57
This revision was automatically updated to reflect the committed changes.
valerio.bozzolan added a commit: rP775d141fe6bf: Audit Feed: less verbose when the author is the committer.
valerio.bozzolan mentioned this in Next Up.Nov 15 2023, 12:58
avivey mentioned this in 2023 week 49.Dec 8 2023, 17:36

Revision Contents
Changeset List

PathSizePackages
src/
applications/
audit/
storage/
12 linesO1: Blessed Committers

Diff 1520

View Options

src/applications/audit/storage/PhabricatorAuditTransaction.php

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0