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(TypeScript): Add the workflow file for CI check #1280

Merged
merged 18 commits into from
Apr 19, 2024
Merged

feat(TypeScript): Add the workflow file for CI check #1280

merged 18 commits into from
Apr 19, 2024

Conversation

Shyam-Chen
Copy link
Contributor

If this pull request (PR) pertains to Chinese-to-English translation, please confirm that you have read the contribution guidelines and complete the checklist below:

  • This PR represents the translation of a single, complete document, or contains only bug fixes.
  • The translation accurately conveys the original meaning and intent of the Chinese version. If deviations exist, I have provided explanatory comments to clarify the reasons.

If this pull request (PR) is associated with coding or code transpilation, please attach the relevant console outputs to the PR and complete the following checklist:

  • I have thoroughly reviewed the code, focusing on its formatting, comments, indentation, and file headers.
  • I have confirmed that the code execution outputs are consistent with those produced by the reference code (Python or Java).
  • The code is designed to be compatible on standard operating systems, including Windows, macOS, and Ubuntu.

@krahets
Copy link
Owner

krahets commented Apr 17, 2024

Hi, I added a bug to test the workflow but it still reported success.

@Shyam-Chen
Copy link
Contributor Author

It seems like executing TypeScript code (test_all.ts) might not be necessary anymore, as the code doesn't utilize assertions, so type checking should suffice. Moving the execution to JavaScript could be an option.

@krahets
Copy link
Owner

krahets commented Apr 19, 2024

Please remove the file changes irrelevant to the CI check to make the semantic of the PR clear.

@krahets
Copy link
Owner

krahets commented Apr 19, 2024

By the way, could you avoid unnecessary force-pushes? It's relatively inconvenient to track the history.

@Shyam-Chen
Copy link
Contributor Author

Shyam-Chen commented Apr 19, 2024

Due to the use of modern ES modules, Node does not yet support import.meta.main, a feature exclusively available in Deno.

https://docs.deno.com/runtime/manual/runtime/import_meta_api#importmetamain

require.main === module
// ->
import.meta.url === `file://${process.argv[1]}`
// ->
import esMain from 'es-main';
esMain(import.meta)
Node Deno
import.meta.url
import.meta.main
import.meta.filename
import.meta.dirname
import.meta.resolve

Copy link
Owner

@krahets krahets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Would you like to add the Javascript's workflow file in a new PR?

@krahets krahets added enhancement Brand new feature code Code-related labels Apr 19, 2024
@krahets krahets merged commit 587344d into krahets:main Apr 19, 2024
3 checks passed
@Shyam-Chen
Copy link
Contributor Author

Shyam-Chen commented Apr 19, 2024

Would you like to add the Javascript's workflow file in another PR?

In JavaScript, only the previously written test_all.ts can be used for modification. Or use allowJs in tsconfig.json?

@krahets
Copy link
Owner

krahets commented Apr 19, 2024

@Shyam-Chen We can do that for JavaScript because it is a relative Ioose programming language. It's acceptable to only find the exception that crashes the program. (Just like Python)

@Shyam-Chen Shyam-Chen deleted the ci branch April 20, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code-related enhancement Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants