Create the spec for create entyr operation and getting initial feedback
Depends on D13315
Details
Details
Flow. Testing later in stack
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/dm-ops/create-entry-spec.js | ||
|---|---|---|
| 75–77 ↗ | (On Diff #44116) | We need to check that the thread exists. |
| lib/types/dm-ops.js | ||
| 354–356 ↗ | (On Diff #44116) | It is a bit confusing to see time next to a date because it feels like they represent two parts of the same timestamp. I think we should make it explicit that the date relates to an entry while time to the operation. |
| lib/shared/dm-ops/create-entry-spec.js | ||
|---|---|---|
| 28 ↗ | (On Diff #44116) | Could you please amend the test plan to check for notifs? In order to test notifs simply flip NEXT_CODE_VERSION to 0 and notifs shoudl work automatically. |
| 75 ↗ | (On Diff #44116) | You use methodName() {} syntext here while you use methodName: () => {} syntax for other methods. Could you please, use consistent approach for all methods in this spec? Ideally you would copy the approach that is used in other specs. |
| lib/shared/dm-ops/create-entry-spec.js | ||
|---|---|---|
| 75 ↗ | (On Diff #44116) | Our general approach on picking between these is usually not about consistency in a single file. We opt for methodName() {} when we don't need methodName: () => {} because it's slightly better for performance. We use methodName: () => {} when we want to pass this.methodName somewhere without losing the bound this variable |
| lib/types/dm-ops.js | ||
|---|---|---|
| 354–356 ↗ | (On Diff #44116) | Actually, this didn't. Creating a follow up for this: https://linear.app/comm/issue/ENG-9307/rename-create-entry-spec-date-field-to-entrydate-to-avoid-confusing |
| lib/shared/dm-ops/create-entry-spec.js | ||
|---|---|---|
| 28 ↗ | (On Diff #44116) | |