FairChange
FairChange copied to clipboard
Create GitHub actions to test any code before merging
Background on the problem the feature will solve/improved user experience
Add GitHub Actions to test and build new code pushed to a branch or committed to a pull request. Code that passes all the steps in the GitHub action can be merged in the repository.
Describe the solution you'd like
A GitHub action runs whenever code is committed to any branch or PR is made to the main branch for each of the following components:
- [ ] website folder
- [ ] mobile folder
- [ ] backend folder
Refer to a similar action that was implemented for the UI folder in Five Fifths Voter project.
Each action will
- [ ] install dependencies
- [ ] build for production
- [ ] run all tests
- [ ] linting with prettier. Discuss with the team on what linter to use.
Tasks
- [ ] Ensure install, build and lint steps run locally on your system
- [ ] Read more about GitHub actions
- [ ] Create an action for each folder. You can read more JavaScript build actions here.
- [ ] Commit and push your GitHub action code.
- [ ] Go to the Actions tab to see if it worked.
Acceptance Criteria
GitHub action runs for every new code commit and PR.
FYI @demilolu
Hi, I would like to work on this issue
@arianahl go for it!
@demilolu the pull request adds the git actions requested. A few things I would like to point out:
- backend and website don't have tests so I left that line commented out
- backend and mobile don't have lint configured so I left that line commented out
- website has lint but it is misconfigured and gives many errors when running
Are these known issues?
@arianahl I think 1 and 2 are, but I'm not sure about 3.
@upkarlidder @Jamstew2 insight here?
For 3, are you able to fix the errors @arianahl. That would be ideal.
@upkarlidder If no one is working on the website part of the code I can give it go. Otherwise they would get a horrible merge on their hands.
~~Also a way to make it less intrusive is to change the current lint rule for parentheses: instead of having them on the next line leave them in the same line as most of the code already does.~~ Actually this is only a problem in the index.js script. The rest of the files do follow allman
style.
I can also go ahead and turn it on for the other two folders (different PR?).
@upkarlidder If no one is working on the website part of the code I can give it go. Otherwise they would get a horrible merge on their hands. ~Also a way to make it less intrusive is to change the current lint rule for parentheses: instead of having them on the next line leave them in the same line as most of the code already does.~ Actually this is only a problem in the index.js script. The rest of the files do follow
allman
style. I can also go ahead and turn it on for the other two folders (different PR?).
It sounds good to me. @Jamstew2 and team can you please provide some guidance as well?
@upkarlidder I fixed the linting issues as part of pull request #62 which adds to #61. I made sure the actions are still passing. Given the many changes I would recommend a thorough review from someone that knows the code just in case. I did test the website locally and I couldn't see any errors.
@kyleni are you able to review the related PRs please?