smhg / gettext-parser

Parse and compile gettext po and mo files, nothing more, nothing less
MIT License
158 stars 43 forks source link

fix missing types and jsDocs #89

Closed erikyo closed 1 month ago

erikyo commented 2 months ago

This pull request addresses the need for consistent documentation within the repository by adding JSDoc comments where they were previously missing.

This enhances code readability and maintainability.

Each file has been reviewed and updated as necessary to ensure comprehensive documentation coverage.

erikyo commented 1 month ago

the last commit was required to show the IDE tooltip / suggestion in the right way (tested vscode and webstorm). replaced the default export with named export for Po* singleton

ide

erikyo commented 1 month ago

I enabled strict types and fails

cc. @johnhooks

erikyo commented 1 month ago

will apply the suggestion asap... thank you John! Anyhow check this:

https://github.com/smhg/gettext-parser/pull/89/checks

there are only about 30 type errors left 🚀 then we are good to go

erikyo commented 1 month ago

The remaining types I sincerely believe require something structural that cannot be done in this pr.

I am referring for example to these which should be classes (https://github.com/smhg/gettext-parser/blob/babf333b3c8b2d617c038ca1b41065cc90f1dc49/src/poparser.js#L590) which has to extend the base class to inherit the types or objects which have the same keys (ie the translation) but they have been used for different ways creating some confusion in the type to be used

what do you think @johnhooks? should I disable the type checking in order to pass again the tests so then we can focus on the last 20-30 issues?

johnhooks commented 1 month ago

@erikyo I think it's fair to set continue-on-error: true on the tsc GitHub action step, and accept that it fails. There might also be other ways to indicate a step as an optional part of the job.

erikyo commented 1 month ago

in this case we should split the CI actions into 2 different steps because otherwise it will allow the tests to fail

erikyo commented 1 month ago

fixed, and fixed the reporter in the code tab (previously was duplicated)