refactor: migrate user-agent packages to TS #1393
Conversation
|
FYI: I will rebase this branch once #1395 has been merged, so that it only contains the packages migration. |
acbeni
left a comment
acbeni
left a comment
There was a problem hiding this comment.
Overall looks good 🎖
there is just a repeated part as mentioned in the comment that i m not sure i ve got quite correctly.
|
the incremental approach sounds good for now, just one additional remark for the header and i think we re good to go 🎖 |
emmenko
force-pushed
the
nm-typescript-setup
branch
from
5d6853e to
6f7c7ad
Compare
emmenko
changed the title
| { | ||
| "files": "*.ts", | ||
| "options": { | ||
| "parser": "typescript" |
There was a problem hiding this comment.
Here we use the override feature to tell prettier to use typescript parser for the ts files, otherwise the babel-flow parser.
| @@ -0,0 +1,41 @@ | |||
| module.exports = { | |||
| parser: '@typescript-eslint/parser', | |||
There was a problem hiding this comment.
As mentioned before, for now TS packages should have their own eslint config, which is merged with the base one.
Once we've migrated all packages, we can merge this back with the base one.
|
Alright, this is now ready for review. /cc @tdeekens for awareness |
emmenko
force-pushed
the
nm-typescript-setup
branch
from
9cd5a1f to
08b6375
Compare
| ) | ||
| }) | ||
| .catch((error: Error) => { | ||
| reject(error) |
There was a problem hiding this comment.
This was missing. Somehow the tests caught this now
| key: 'my-state', | ||
| } | ||
| expect(csvParser._transformTransitions(actual)).resolves.toEqual(actual) | ||
| return expect(csvParser._transformTransitions(actual)).resolves.toEqual( |
There was a problem hiding this comment.
There were a bunch of tests that did not return the expectation with the promise.
Codecov Report
@@ Coverage Diff @@
## master #1393 +/- ##
==========================================
+ Coverage 98.73% 98.74% +<.01%
==========================================
Files 121 123 +2
Lines 3167 3176 +9
Branches 728 729 +1
==========================================
+ Hits 3127 3136 +9
Misses 36 36
Partials 4 4
Continue to review full report at Codecov.
|
|
Alright, CI is green now. I'll leave this open for the rest of the day and merge it tomorrow. Then we can continue migrating packages step-by-step. |
| import { CartDiscountPagedQueryResponse } from './../../models/CartDiscount' | ||
| import { CartDiscount } from './../../models/CartDiscount' | ||
| import { Middleware } from './../../base/common-types' | ||
| import { ApiRequest } from './../../base/requests-utils' |
There was a problem hiding this comment.
@acbeni fixing these looks like something we should do in the generator :-)
Setup PR:
For now I migrated 2 small packages:
http-user-agentsdk-middleware-user-agentI also added a new package
sdk-types, which contains shared TS types (e.g. middleware).