wikibase-sdk icon indicating copy to clipboard operation
wikibase-sdk copied to clipboard

fix and improve types

Open EdJoPaTo opened this issue 2 years ago • 5 comments

This touched a lot of things again. I enabled some more tsconfig settings and removed some //@ts-nocheck. The only one left is the simplify_claims test one which seems especially fun to get rid of in the future. I also moved time related stuff into wikibase_time.ts.

@mshd When you have time to review this one it would be nice :)

EdJoPaTo avatar Feb 23 '23 20:02 EdJoPaTo

I got a bunch of claim parsing typed which is nice. On the downside: I'm not sure how much I like the simplify methods as they are very dependent on the input and their options. Especially with types its simpler to pick the exact type you want and simplify exactly for your needs rather than simplifying with some options while loosing typings. I would consider publishing the parse_claim methods and deprecating the simplify claim related methods.

EdJoPaTo avatar Feb 27 '23 19:02 EdJoPaTo

This is horrible to review… Maybe I should split this up into multiple Pull Requests which can be reviewed and accepted on their own.

I keep this one open and rebase it on top of the current main branch as a "progress" indication and provide Pull Requests for parts. Then I'll wait with more changes until they are merged to prevent a huge clogged up PR like this one. Does that sound good to you @maxlath ?

EdJoPaTo avatar Mar 14 '23 10:03 EdJoPaTo

yes, more atomic PRs would be welcome indeed :sweat_smile:

maxlath avatar Mar 17 '23 09:03 maxlath

@EdJoPaTo can we close this PR now that it was taken over in other PRs?

maxlath avatar Jun 10 '23 11:06 maxlath

There are still some things left which are not added in their own PR… I scrolled over this PR and found these:

  • [ ] eslint → eslint-with-typescript (probably should be done when there are no other open PR to have less breaking changes
  • [ ] src/queries/get_reverse_claims.ts has improved types
  • [ ] claim DataType as an explicit list rather than derived from existing parsers
  • [x] simpler types of src/utils/build_url.ts (see #123)
  • [ ] ts-nocheck is removed on tests/simplify_sitelinks.ts
  • [ ] ts-nocheck is removed on tests/simplify_sparql_results.ts (see #129)

EdJoPaTo avatar Jun 14 '23 10:06 EdJoPaTo