Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nodejs): check for .mts and .cts files #3734

Merged
merged 9 commits into from Mar 12, 2022
Merged

Conversation

kidonng
Copy link
Contributor

@kidonng kidonng commented Mar 11, 2022

Description

These are TypeScript counterparts of Node.js .mjs and .cjs file extensions.

Tests were added but removed later since they are unnecessary and are probably going to be dropped in the future, see conversations below.

Motivation and Context

Closes #

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@kidonng kidonng marked this pull request as ready for review March 11, 2022 05:25
@kidonng
Copy link
Contributor Author

kidonng commented Mar 11, 2022

Leaving some random thoughts here:

  • Ideally we should generate some metainfo file from src/configs and use that as data source in the docs instead. Single source of truth is better and avoid manual sync (and mistakes).
  • Tests for detect_extensions, detect_files and detect_folders are way more than necessary. We should only have one test for each of their functionality.

@davidkna
Copy link
Member

Ideally we should generate some metainfo file from src/configs and use that as data source in the docs instead. Single source of truth is better and avoid manual sync (and mistakes).

Case in point: you need to update detect_extesions in the table. Would you mind creating an issue about this? It might be possible to follow whatever rustdoc is doing or maybe a JSON-schema could be used as a data source (#1951).

Tests for detect_extensions, detect_files and detect_folders are way more than necessary. We should only have one test for each of their functionality.

True, there's not much value in that. I feel like a macro could handle most of those tests anyway.

@kidonng
Copy link
Contributor Author

kidonng commented Mar 11, 2022

Would you mind creating an issue about this?

If there's an issue open for that then it's good enough, we need to have the schema first anyway.

@andytom andytom merged commit a10e24b into master Mar 12, 2022
@andytom andytom deleted the nodejs-extensions branch March 12, 2022 22:09
@andytom
Copy link
Member

andytom commented Mar 12, 2022

Thank you for your contribution @kidonng and thanks for the review @davidkna.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants