-
Notifications
You must be signed in to change notification settings - Fork 0
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/68 Initialize Backend #116
Conversation
.vscode/settings.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't committed vscode settings in the past...is it a team requirement to use it / to abide by those settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was something that was added through on the cas-registration side. True, I usually have to merge my local settings with the ones coming out of the repo 😆. Might be better added as in the document for a developer to add themselves if needed.
|
||
## Pre-Commit | ||
|
||
TODO: Add pre-commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: precommit CI stuff is awaiting review here https://github.com/orgs/bcgov/projects/123/views/2?pane=issue&itemId=49861453
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fantastic! Just a couple of comments for file names
bc_obps/.env.example
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have the root folder for the server/api side be server
or api
? bc_obps
might be confusing and have devs mix things up with the registration app
edit: comment below, maybe bc_obps_reporting_api
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this over in 72b568b.
This leads to the question of if/how we'll integrate Django work on Reporting with the deployed Registration server. It's not something I'm familiar with, so I don't even know where to start with it. Probably worth flagging for discussion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the first setup, let's not get complicated, but agreed, we need to have that discussion sooner than later
8177751
to
fe98626
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last minor change! fantastic work 👌
server/bc_obps/__init__.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the django app is still named bc_obps
- which is the same as the registration app, so we'll have a conflict if we ever run them off the same server. We should probably rename that to bc_obps_reporting_api
too
72b568b
to
50a71a7
Compare
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. |
50a71a7
to
1458e3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
Addresses #68 - Project link
Adds Terraform scripts and Helm charts to run them. Resource states migrated from Terraform Cloud to a Google Cloud Storage bucket.
Changes 🚧
To test 🔬
docs/developer-environment-setup.md
and follow the directions to set up your developer environment to set up the backend.localhost:8000/api/docs
. It should show the Ninja API documentation for add.localhost:8000/api/add?a=4&b=2
endpoint in your browser. It should display the result of the addition.Notes📝
cas-registration
. Changes that might have been made later in the backend should be reflected here where it makes sense to get them in at the start.