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

Add GitHub Actions #499

Merged
merged 4 commits into from
Aug 29, 2024
Merged

Add GitHub Actions #499

merged 4 commits into from
Aug 29, 2024

Conversation

phanect
Copy link
Contributor

@phanect phanect commented Aug 24, 2024

This PR adds configs for GitHub Actions.
Currently, it verifies if the build and check (with @astro/check, ESLint, and Prettier) are successfully done.
I also fixed errors and warnings by the linters.

  • Currently, GitHub Actions runs with Node.js v18, v20, and v22. I found AstroWind also supports v21 according to engine.node in package.json, and should we test with v21 on the CI?
  • If the naming for npm scripts does not match your preference, let me know how I should fix it, or feel free to add commits to fix it.
  • This PR includes changes for package-lock.json. Since it would probably conflict with Convert JavaScript files to TypeScript #501, please regenerate package-lock.json on your end, or let me regenerate it if it is required.
  • You might want to overwrite package-lock.json on your end to ensure I'm not an attacker to inject malicious code in package-lock.json

@phanect
Copy link
Contributor Author

phanect commented Aug 28, 2024

I have rebased this branch to resolve the conflict.

@prototypa
Copy link
Contributor

Hi @phanect

Thank you very much for this.

Question: why do you repeat the check job with version 22 if you have already executed that verification in the build job?

@phanect
Copy link
Contributor Author

phanect commented Aug 28, 2024

Thank you for the review, @prototypa.

I guess astro build does not check everything checked by astro check. It seems to run only some checks.

I will check the behavior of astro build and astro check again.

@phanect
Copy link
Contributor Author

phanect commented Aug 28, 2024

@prototypa I have confirmed that astro build does not check the typing error of TypeScript while astro check does.
Therefore, both astro build and astro check should run on the CI, IMO.

astro build returns 0 on the shell while astro check returns 1 because there is unmatched error in the TypeScript code

@phanect
Copy link
Contributor Author

phanect commented Aug 28, 2024

No, sorry, I have misunderstood what you mean.
I just noticed I wrote npm run check both in the build and check jobs of the GitHub Actions configs.
That's just a mistake. I will fix it.

@prototypa
Copy link
Contributor

Hi @phanect

Thank you so much! Let's approve this PR.

@prototypa prototypa merged commit 4254a60 into onwidget:main Aug 29, 2024
@phanect phanect deleted the feat-gha branch August 29, 2024 21:46
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.

2 participants