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
- Branch
- thick-threaded-entries
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/dm-ops/create-entry-spec.js | ||
|---|---|---|
| 75–77 | We need to check that the thread exists. | |
| lib/types/dm-ops.js | ||
| 354–356 | 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 | 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 | 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 | 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 | 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 | ||