Conversation
keefertaylor
requested review from
amiecorso,
sarahkevinking and
tedkalaw
as code owners
| return { Memos: convertedMemos } | ||
| memoListToJSON(memos: Memo[]): MemoJSON[] { | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method -- Manually assigning `this`. | ||
| return memos.map(this.memoToJSON, this) |
There was a problem hiding this comment.
can you use an arrow function instead here?
return memos.map((memo) => this.memoToJSON(memo))
this is preserved in arrow functions
There was a problem hiding this comment.
Totally. I was trying to make the linter happy here (we do this other places too).
Do we have any thoughts on whether we should disable this linter rule?
There was a problem hiding this comment.
it gets mad with arrow functions? the dox for the rule implies that arrow funcs are cool: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md
if we're doing this other places already, i say just go for it as is
|
+ Hans who may have comments about the linter. I think you've mentioned we could disable the rule in question in the past, I just forget exactly what you've said :) |
|
Gentle ping, @tedkalaw |
Codecov Report
@@ Coverage Diff @@
## master #528 +/- ##
==========================================
- Coverage 92.01% 92.00% -0.02%
==========================================
Files 11 11
Lines 676 675 -1
Branches 161 161
==========================================
- Hits 622 621 -1
Misses 28 28
Partials 26 26
Continue to review full report at Codecov.
|
| return { Memos: convertedMemos } | ||
| memoListToJSON(memos: Memo[]): MemoJSON[] { | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method -- Manually assigning `this`. | ||
| return memos.map(this.memoToJSON, this) |
There was a problem hiding this comment.
it gets mad with arrow functions? the dox for the rule implies that arrow funcs are cool: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md
if we're doing this other places already, i say just go for it as is
3 participants
High Level Overview of Change
Refactor memo list to be standardized.
Context of Change
Refactor method to use same idioms as other list serialization methods (see for example,
pathListToJSON).Specifically:
memoListToJSONfor consistencyArray<Memo>and output isArray<MemoJSON>Type of Change
Before / After
No change. This is prep work for a larger refactor of common transaction fields in #519
Test Plan
CI shows that this does not affect the public API; removed a unit test which was testing removed functionality.