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

chore: add quickstart template #71

Merged
merged 34 commits into from
Dec 2, 2024
Merged

chore: add quickstart template #71

merged 34 commits into from
Dec 2, 2024

Conversation

marcellmueller
Copy link
Contributor

@marcellmueller marcellmueller commented Nov 26, 2024

WIP

Setting up project using https://github.com/bcgov/quickstart-openshift modified to comment/remove OpenShift specific features.

Todo:

  • Request Renovate setup
  • Request SonarCloud tokens
    • Add SonarCloud tokens to secrets
  • Delete charts directory
  • Comment deployment workflows and remove OpenShift specific jobs and secrets
  • Update Readme.md

Todo after merge to main:

  • Setup SchemaSpy gh-pages
  • Add Trivy to branch Ruleset code scanning results
  • Add required status checks:
    • Analysis Results
    • PR Results
    • Validate Results

Notes:

  • Replaced jest with Vitest in the /backend project 🎉
  • Added pre-commit hooks
  • Added bcparks.ca bootstrap theme. The BCGov React component library seems awesome so leaving it in the project for now, though I think we will have to mostly use the bootstrap theme for consistency since we are aligning with Parks.
  • Removed MUI completely - absolutely nothing against MUI, though I'm not planning on using MUI box for styling so we can always add back if we decide we need a helpful library such as the DataGrid.
  • For now we can use vanilla CSS modules for styling. We will likely be using the bootstrap theme which uses scss, though the Vite docs recommend PostCSS plugins if we need to extend vanilla CSS features.

Eslint

Configuring eslint to work on the monorepo and not just the frontend ended up being a big pain. I think it's mostly working correctly now 🤞

  • ESLint 8 is EOL so I migrated to eslint 9 which uses flat config
  • Added eslint config in root of project
  • Frontend extends that config with React specific plugins and rules
  • Added eslint pre-commit hook
  • Perhaps in the future we will want to add an eslint config to the /backend project. Though I think using the base config is fine for now.

@marcellmueller marcellmueller force-pushed the 51-setup-git-repo branch 4 times, most recently from 6051cd0 to 338b462 Compare November 26, 2024 17:12
@mishraomp
Copy link
Collaborator

great going @marcellmueller , Looking forward to steal some items, vitest for backend, precommit hooks

cc @DerekRoberts

@marcellmueller marcellmueller force-pushed the 51-setup-git-repo branch 3 times, most recently from 661c988 to 21ec8ed Compare November 29, 2024 23:55
@mishraomp
Copy link
Collaborator

@marcellmueller thoughts on https://typicode.github.io/husky/ for precommit hooks, I have seen other teams use it, though I have to say, I have not used it personally.

Any teams in FDS using it @DerekRoberts ?

@DerekRoberts
Copy link
Member

DerekRoberts commented Dec 1, 2024

I'm not a fan of precommit hooks because they impede frequent pushing and updating. People can circumvent with '--no-verify' too. Letting the CI (workflow) handle it is ideal! We have several examples.

Over here only the capstone project, which was all students, has suffered through Husky. @marcellmueller @mishraomp

@mishraomp
Copy link
Collaborator

I'm not a fan of precommit hooks because they impede frequent pushing and updating. People can circumvent with '--no-verify' too. Letting the CI (workflow) handle it is ideal! We have several examples.

Over here only the capstone project, which was all students, has suffered through Husky. @marcellmueller @mishraomp

well I take my words back rip husky

@DerekRoberts
Copy link
Member

@mishraomp Lol! It's not all bad, just maybe a little remedial. :D + :P

@marcellmueller
Copy link
Contributor Author

I'm not a fan of precommit hooks because they impede frequent pushing and updating. People can circumvent with '--no-verify' too. Letting the CI (workflow) handle it is ideal! We have several examples.

Over here only the capstone project, which was all students, has suffered through Husky. @marcellmueller @mishraomp

Well it might be a good idea to leave it out of the OpenShift quickstart though I think pre-commit hooks are very useful. Without it the team regularly commits messy or even broken code and as someone who isn't a fan of nit-heavy PR reviews having eslint properly implemented with pre-commit helps mitigate that.

I think the quickstart template is a good example of this, as it has numerous linting errors once eslint was fixed and formatting inconsistencies ;)

Using the --no-verify flag is also a valid workflow, you can always run pre-commit run --all-files on your PR before review so it will pass CI.

@DerekRoberts
Copy link
Member

@marcellmueller Given that arugment, sure! It's easier for teams to remove features than add, so we could be helping teams by annoying them. :D ...the linting errors point more to us needing help rather than any high-level decision making. :/ @mishraomp

@DerekRoberts
Copy link
Member

@marcellmueller Can you be conned into providing these improvements to the QuickStart? We'd most definitey appreciate it. :) @mishraomp

@marcellmueller
Copy link
Contributor Author

@marcellmueller Can you be conned into providing these improvements to the QuickStart? We'd most definitey appreciate it. :) @mishraomp

I'd love to help with some PRs. I'll make sure my PO is good with me spending a little time contributing back. Would love to prioritize getting eslint working for the entire project and Vitest as the backend test runner for consistency if that is okay :)

@DerekRoberts
Copy link
Member

DerekRoberts commented Dec 2, 2024

@marcellmueller That'd be great, thanks! ...worst case we can just "liberate" your ideas using good old theft. :D + :P

@marcellmueller marcellmueller marked this pull request as ready for review December 2, 2024 22:11
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Copy link
Collaborator

@mishraomp mishraomp left a comment

Choose a reason for hiding this comment

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

nicely done, lgtm

@marcellmueller
Copy link
Contributor Author

@marcellmueller That'd be great, thanks! ...worst case we can just "liberate" your ideas using good old theft. :D + :P

Hey I tried to open a PR for replacing Jest with Vitest in the backend, however I don't have access to create a branch. Can I get access to do that?

@DerekRoberts
Copy link
Member

@marcellmueller That'd be great, thanks! ...worst case we can just "liberate" your ideas using good old theft. :D + :P

Hey I tried to open a PR for replacing Jest with Vitest in the backend, however I don't have access to create a branch. Can I get access to do that?

@marcellmueller Sorry, you now have write access! Thanks, we're looking forward to this. :) @mishraomp

@marcellmueller marcellmueller merged commit da82e4b into main Dec 2, 2024
10 checks passed
@marcellmueller marcellmueller deleted the 51-setup-git-repo branch December 2, 2024 23:35
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