Page MenuHomePhabricator

[lib] Use slanted apostrophes and quotation marks in `change-settings-message-spec`
AcceptedPublic

Authored by abosh on Sep 2 2022, 12:22 PM.
Tags
None
Referenced Files
F3364555: D5040.diff
Mon, Nov 25, 4:14 AM
Unknown Object (File)
Sat, Nov 16, 3:20 PM
Unknown Object (File)
Mon, Nov 11, 2:51 AM
Unknown Object (File)
Fri, Nov 8, 8:58 AM
Unknown Object (File)
Fri, Nov 8, 6:37 AM
Unknown Object (File)
Sat, Nov 2, 1:15 PM
Unknown Object (File)
Sat, Nov 2, 1:15 PM
Unknown Object (File)
Sat, Nov 2, 1:15 PM
Subscribers

Details

Reviewers
atul
tomek
Summary

Quick change I noticed before working on ENG-1733. This uses apostrophes and quotation marks that are slanted instead of the straight ones since this is user-displayed text. Also removes string concatenation from return value of robotext(...) since it's slower.

Test Plan

Flow, close reading, just replaced characters / removed concatenation.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

adjusted my herald rules to hopefully not add me on lib/shared revisions

tomek requested changes to this revision.Sep 5 2022, 1:20 AM

Is it consistent with all the usages in the app? If we decide to use slanted signs, we should make sure to use the same version through the app.

This revision now requires changes to proceed.Sep 5 2022, 1:20 AM

I agree this is a good change, and I think we could land this if we create a starter task for somebody else to find all of the other locations this occurs in the codebase

abosh retitled this revision from [lib] Use apostrophe and quotation marks in `change-settings-message-spec` to [lib] Use slanted apostrophes and quotation marks in `change-settings-message-spec`.Sep 14 2022, 4:25 PM
abosh removed a reviewer: jon.

Addressed @tomek's feedback:

Is it consistent with all the usages in the app? If we decide to use slanted signs, we should make sure to use the same version through the app.

Turns out I had actually tried tackling this in D4541, the only issue being the regex I used to match was ".*'.*", and I forgot to also match `.*'.*` (strings that are encapsulated with backticks).

Fixed all usages of ' and "" in the app by searching using the regex with backticks to find all strings with backticks that contained ' or "" symbols in user displayed text. Replaced each usage with slanted counterparts.

In D5040#150730, @abosh wrote:

Addressed @tomek's feedback:

Is it consistent with all the usages in the app? If we decide to use slanted signs, we should make sure to use the same version through the app.

Turns out I had actually tried tackling this in D4541, the only issue being the regex I used to match was ".*'.*", and I forgot to also match `.*'.*` (strings that are encapsulated with backticks).

Fixed all usages of ' and "" in the app by searching using the regex with backticks to find all strings with backticks that contained ' or "" symbols in user displayed text. Replaced each usage with slanted counterparts.

I think you should also search for '.*\'.*' - I would be surprised if we have things like this, but it's better to be sure.

This revision is now accepted and ready to land.Sep 16 2022, 10:45 AM