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

Migrate from Travis CI to Github Actions #35

Merged
merged 11 commits into from
Mar 3, 2023

Conversation

Jooms
Copy link
Contributor

@Jooms Jooms commented Mar 2, 2023

This resolves some of the tasks in #19

  • Add a GitHub Action workflow to verify a successful build of the project. Should be triggered by all pull requests.

Note:
I had trouble figuring out how to just build a node project. It seems that since Javascript is interpreted (rather than compiled), all we can do is run it. So I added a flag to the starting file that short-circuits the code just before the server starts (and tries to connect to other things).

If there are better ideas, let me know.

@Jooms
Copy link
Contributor Author

Jooms commented Mar 2, 2023

@kbuffardi I don't see an option on my end yet, but I'm assuming you can please squash + merge this. If not, please let me clean up the commits before merging.

Copy link
Contributor

@jayrevolinskyjr jayrevolinskyjr left a comment

Choose a reason for hiding this comment

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

Is there a way to guarantee that static analysis occurs separate from the start-and-kill?

My only concern is that it might try to run the build and analysis together. I believe that's the case but I am uncertain. Despite this, LGTM.

@Jooms
Copy link
Contributor Author

Jooms commented Mar 2, 2023

Is there a way to guarantee that static analysis occurs separate from the start-and-kill?

My only concern is that it might try to run the build and analysis together. I believe that's the case but I am uncertain. Despite this, LGTM.

Great question.

Just a clarification.
The npm ci here is just clean install the project. It's an alternative to npm install that is meant to run in pipelines because it adds a small amount of extra validation for third-party dependencies. https://docs.npmjs.com/cli/v9/commands/npm-ci

So in this case, we're doing a little bit of static analysis of the dependencies, but often if those are broken the build will fail. So I'd argue it makes sense in this case.

Copy link
Contributor

@reembot reembot left a comment

Choose a reason for hiding this comment

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

LGMT outside of the .yml filename. If we're expecting multiple workflow yamls, naming this something specific to its purpose could be helpful?

Copy link
Contributor

@reembot reembot left a comment

Choose a reason for hiding this comment

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

LGTM

@Jooms Jooms merged commit e9e026d into ChicoState:main Mar 3, 2023
@Jooms
Copy link
Contributor Author

Jooms commented Mar 3, 2023

Ugh sorry for the commit spam. I thought I selected squash merge

briswells pushed a commit that referenced this pull request Mar 23, 2023
This resolves some of the tasks in #19 
- [ ] Remove all dependencies on Travis CI
- [x] Add a GitHub Action workflow to verify a successful build of the
project. Should be triggered by all pull requests.
- [ ] Add a GitHub Action workflow to perform static analysis of all
pull requests.
- [ ] Add a GitHub Action status badge to README.md with the current
build status of the main branch using the new CI workflow

**Note:**
I had trouble figuring out how to **just** build a node project. It
seems that since Javascript is interpreted (rather than compiled), all
we can do is run it. So I added a flag to the starting file that
short-circuits the code just before the server starts (and tries to
connect to other things).

If there are better ideas, let me know.
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.

3 participants