Conversation
TomJo2000
left a comment
There was a problem hiding this comment.
Just have a couple questions.
| /**@type {unknown}*/ | ||
| const repos = JSON.parse( |
There was a problem hiding this comment.
This is a JSON object corresponding to https://github.com/termux/termux-packages/blob/master/repo.json
There was a problem hiding this comment.
Thanks! I had replaced the any by unknown because there's no absolute guarantee that the input repo.json honors the schema. But I guess it's fine to make some assumptions there
| binMap.set(packageName, []); | ||
| } | ||
| binMap.get(packageName).push(binary); | ||
| /**@type {string[]}*/ (binMap.get(packageName)).push(binary); |
There was a problem hiding this comment.
That seems like a messy way to express that type annotation.
thunder-coding
left a comment
There was a problem hiding this comment.
I'm not a fan of package-lock.json changes being committed by contributors. I'd prefer if it is generated by one of the organization members for security reasons.
Also for the type hints, I think it's better to just switch to TypeScript than add typehints, I find them pretty annoying to deal with and would prefer switching to TS. Also if we want to switch to TS, I find this script to be too small to actually be worth it for switching to TypeScript. It's just 272 lines of JavaScript out of which 51 are comments. Would like to know about the opinion of others on this
- add some assertions - minor refactor - add JSDoc: comments, type-annotations, and type-casts - `npm i '@types/node@latest' prettier@latest typescript@latest` - strict `jsconfig.json` - add some props to `package.json`
I enabled type-checking and specified which version of TypeScript and Prettier are being used.
I'm considering to remove the
jsconfigin favor of a simple//@ts-checkdirective