[GitHub Actions] Set up GitHub Actions to automatically build, test, and format #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pretty much just default Node.js config, just changed to only run on PRs (instead of pushes and PRs targetting "master")
|
Disregard for now, I meant to open this against my own branch. Since the PR is already here, I'll leave it as a draft, but it's not ready for review yet. |
package.json defines requirement node >= 16
No idea if this will Just Work (TM), but I guess we'll find out soon!
GH concats it all into one line so escaping the line breaks isnt needed
This uses incorporates mattallty/jest-github-action to add annotations in the test files on the failing tests, making it easier to identify the problems
Test and Format now rely on Build, reducing duplicate code (if this works the way I think it will)
Last commit: > Test and Format now rely on Build, reducing duplicate code > (if this works the way I think it will) It did not. They're still unique environments, so it still has to be set up for each action
Intentionally failing a test and messing up file formatting to see how the actions deal with that
I won't tell it to actually commit yet, first I'll review the output to make sure it isn't committing too much
This reverts the failing test intentionally added in ddc6ce6
I won't tell it to actually commit yet, first I'll review the output to make sure it isn't committing too much
WeirdAlex03
changed the title
|
This is now done, and I have updated the PR for review. Full details in the main comment at the top. As mentioned there, an example of the feedback can be found on PR #1 on my fork. |
jessehartloff
approved these changes
Oct 26, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
This adds a GitHub Actions configuration to automatically verify the project builds, run tests, and run the formatter on pull requests, and share results. An example of the feedback can be found on PR #1 on my fork. You can open new PRs on my fork to test other cases.
A handful of files were not formatted going into this, so the formatting changes on those are also included in this PR.
Closes #22
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...
This only implements checks that are already part of the project. More actions can be added to the workflow for Code Scanning to automatically detect and report potential security vulnerabilities, or anything else we might want (more about code scanning). I can add them in this PR, if that is wanted.
I also suggest considering a branch protection rule to require some or all tasks to pass before merging is allowed (permissions can be set to allow some/all administrators to bypass this, if wanted).
Lastly, also notable is the low test coverage across many files, as noted in the comment seen on my PR (which can be disabled, btw). I believe we should make it a point to review and improve all of the test suites to ensure there is adequate coverage, both simply in terms of lines of code, and in terms of logical edge/corner cases.