ajuanjojjj commented on PR #588: URL: https://github.com/apache/cordova-plugin-file/pull/588#issuecomment-1656137882
About the tests; I wouldn't mind at all if d.ts files would be moved to DefinitivelyTyped, but I should note that the [complete](https://www.npmjs.com/package/@types/cordova-plugin-file?activeTab=versions) [opposite](https://github.com/apache/cordova-plugin-file/commit/3abbdee693862b4509f5767ea9bad83259845aed) was done 6 years ago, deprecating the DefinetivelyTyped package in favour of shipping the types with the plugin. Instead, adding tests to the package, if done properly, would help to both keep the d.ts files up to date, and help contributing users by providing some more assurances that their changes are ok, since, at the moment, the only tests are a simple `eslint` pass. You already need to install eslint globally to run said tests (called without npx, missing from devDependencies) and also run npm install to get "@cordova/eslint-config", so I don't think also adding typescript to the dependencies (and probably eslint if we're at it) is that tough of an ask. Still, it might be better to discuss it on a different issue, since although its related to the PR, its a considerably different thing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
