Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/messages/add-members-message-spec.js | ||
|---|---|---|
| 238 ↗ | (On Diff #45779) |
Shouldn't the condition be inversed? When we return null here, the timestamp isn't bumped, so we should return null for thick threads. Is it correct? |
| lib/shared/messages/add-members-message-spec.js | ||
|---|---|---|
| 238 ↗ | (On Diff #45779) | Yes, you're absolutely right. I probably should've tested this better... |
| lib/shared/messages/add-members-message-spec.js | ||
|---|---|---|
| 238 ↗ | (On Diff #45779) | Wait no, this behavior is actually correct. We want to match behavior with showInMessagePreview, which hides for thin threads but shows for thick threads. Similarly, here we want to skip bumping for thin threads, but continue bumping for thick threads. |
| lib/shared/messages/add-members-message-spec.js | ||
|---|---|---|
| 238 ↗ | (On Diff #45779) | Ok, that makes sense! But in that case, the diff's title should be updated. |
| lib/shared/messages/add-members-message-spec.js | ||
|---|---|---|
| 238 ↗ | (On Diff #45779) | You're right – updated. |