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

Split GitHub workflows #8096

Merged
merged 14 commits into from
Jun 14, 2024
Merged

Split GitHub workflows #8096

merged 14 commits into from
Jun 14, 2024

Conversation

Carlgo11
Copy link
Member

@Carlgo11 Carlgo11 commented Jun 14, 2024

This PR splits the repository.yml workflow into separate workflows: one for pull requests containing different tests and one for deployment.

I also started a Node.js job inside the pull requests workflow. My idea is to gradually rewrite all the tests in JavaScript for better performance and easier collaboration as more people know JavaScript than Ruby

I have tested tests/languages.js independently but I'm unsure if I've missed something in the workflows.

Edit: Obviously, the workflow wasn't functioning when I made the PR. It should work now. Sorry for the commit spam 😅

@Carlgo11 Carlgo11 added the enhancement Issue/PR contains enhancements to the overall code of the site. label Jun 14, 2024
@Carlgo11 Carlgo11 marked this pull request as ready for review June 14, 2024 02:07
@hkamran80
Copy link
Member

If you want to keep the Test PR #XXXX run name, I think it'll work if you wrap it in quotes.

run-name: "Test PR #${{ github.event.number }}"

@Carlgo11
Copy link
Member Author

Carlgo11 commented Jun 14, 2024

@hkamran80 You think that will work?
Also, sorry about the commit spam! I tried to do this with the act CLI but it didn't work 🙈

@hkamran80
Copy link
Member

hkamran80 commented Jun 14, 2024

I think so. The problem with the original method is that it was being treated as a comment. Wrapping it in quotes should prevent YAML from recognizing it as a comment.

No worries about the commits!

@Carlgo11
Copy link
Member Author

It did work! 🥳

@hkamran80
Copy link
Member

hkamran80 commented Jun 14, 2024

Do you want to merge this PR now and convert the rest of the tests later or convert them all now and merge this after?

I can help with the conversion if you'd like.

@Carlgo11
Copy link
Member Author

Let's merge it and then convert more of the tests.

I can help with the conversion if you'd like.

I'd love some help! 😅 We can discuss it on Slack.

@hkamran80
Copy link
Member

Sounds good!

@hkamran80 hkamran80 merged commit f3b2c62 into 2factorauth:master Jun 14, 2024
3 checks passed
@Carlgo11 Carlgo11 deleted the nodejs-tests branch June 14, 2024 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR contains enhancements to the overall code of the site.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants