SSC: Fix invite date formatting#62858
Conversation
chrsmith
left a comment
There was a problem hiding this comment.
This PR looks great, and I am happy to see you added unit tests for the new functionality.
I'm just blocking the approval because of that excessively complicated one-liner. If we need to have a larger conversation about how many language features in a single line of code is too many, we can have that. But in short, we should optimize for making our code easier to understand and maintain. Getting fancy with syntax or language features is counter to that goal. And even if there were efficiency gains to be made in that style, I'd still maintain that the increased complexity and cost of maintenance is not worth it.
chrsmith
left a comment
There was a problem hiding this comment.
Thank you for making the suggested changes. 🙏
There was a problem hiding this comment.
Since we already have the if (sentAt) the TypeScript compiler knows that the value is truth, and won't give us grief that the type could be null right?
So can't we just write this as:
| return intlFormatDistance(sentAt || '', now ?? new Date()) | |
| return intlFormatDistance(sentAt, now ?? new Date()) |
ca2fd91 to
66e371b
Compare
This is a step toward https://github.com/sourcegraph/self-serve-cody/issues/725
The list of invitations used to look like this:
Now it looks like this:
which intends to match this design.
Some other examples with the current solution are:
@rrhyne, is this format fine?
Test plan