Feed Differential retitled: do not repeat the title twice
ClosedPublic
Actions

Authored by valerio.bozzolan on Mar 25 2023, 08:17.

Details

Reviewers

avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15183: The Feed "Differential retitled" Feed is too much verbose

Commits

rP42a8b73adbaa: Feed Differential retitled: do not repeat the title twice

Summary

This is a cosmetic modification for a specific Feed.

https://we.phorge.it/T15183

If you retitle a Diff (and you do nothing else), its Feed
is somehow too much verbose:

... retitled NEW_TITLE from OLD_TITLE to NEW_TITLE

In this specific case, the Feed now becomes shorter so that
the NEW_TITLE is not repeated twice:

... retitled NEW_TITLE from OLD_TITLE

Note that, if the title changes again, the Feed naturally comes
back as before so to mention the change and obviously without
any repetition. Example:

... retitled CURRENT_TITLE from OLD_TITLE to NEW_TITLE

Closes T15183

Test Plan

take a Diff named "OLD_TITLE" and retitle to "NEW_TITLE"
visit the homepage and see "retitled NEW_TITLE from OLD_TITLE" (this means the new behavior works)
retitle the Diff from "NEW_TITLE" to "CURRENT_TITLE"
visit the homepage again and see "retitled CURRENT_TITLE from OLD_TITLE to NEW_TITLE" (this means the old behavior still works)

Diff Detail

Repository

rP Phorge

Branch

T15183-feed-retitled

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 180
Build 180: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Mar 25 2023, 08:17

Owners added a reviewer: O1: Blessed Committers.Mar 25 2023, 08:17

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptMar 25 2023, 08:17

Harbormaster completed remote builds in B179: Diff 391.Mar 25 2023, 08:17

make the damn linter happy

Harbormaster completed remote builds in B180: Diff 392.Mar 25 2023, 08:18

simplified inline comment

Harbormaster completed remote builds in B181: Diff 393.Mar 25 2023, 08:23

avivey subscribed.Mar 25 2023, 10:13

avivey added inline comments.

src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php
29	don't we use strcmp to compare strings?

valerio.bozzolan added inline comments.Mar 25 2023, 10:45

src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php
29	Interesting. Why?

valerio.bozzolan added inline comments.Mar 25 2023, 10:51

src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php
29	I mra
29	I'm quite sure in this specific case the `===` operator is the right one to verify if two strings have the very same binary value. AFAIK the strcmp was introduced to handle other cases where you need to understand the comparison difference (greater, equals, less than). https://stackoverflow.com/a/3333369/3451846

avivey accepted this revision.Mar 25 2023, 12:34

avivey added inline comments.

src/applications/differential/xaction/DifferentialRevisionTitleTransaction.php
29	TBH, I don't remember the reasoning. Possibly, because there are different ways to write "the same" string, and the string functions handle those? I agree about the same-binary-value argument in this particular case, tough.