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.

Sign up for GitHub

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

Jump to bottom

Create Database, Verify classes #32

Merged
merged 9 commits into from
Oct 14, 2021
Merged

Create Database, Verify classes #32

merged 9 commits into from
Oct 14, 2021

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Oct 7, 2021 •
edited
Loading

closes #29

  • move Database class, Verify class into their own files
  • add very basic Verify route spec
  • creates Settings file that has a list of configs
  • create and use Settings variables for hard-coded strings/booleans/numbers

@machikoyasuda machikoyasuda added this to the October 2021 milestone Oct 7, 2021
@machikoyasuda
refactor(database): move Database into own file
86df427 
* move Database class into its own file
* add very basic Verify route spec
* update imports
@machikoyasuda
chore: omit
30a3ecf
@machikoyasuda
refactor(app): move Verify into own file
8834930 
* moves Verify class into its own file
* stubs a line that requires app - could not import app
@thekaveman
Copy link
Member

One suggestion is to refactor a configuration module or similar, that can be used by other modules that need it. This is likely what #30 leads to as well.

@machikoyasuda machikoyasuda marked this pull request as ready for review October 12, 2021 23:44
thekaveman
thekaveman requested changes Oct 13, 2021
View reviewed changes
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Some minor formatting stuff, trying to be consistent even where our pre-commit hooks fail us.

Small suggestion to move some additional settings out of app.

thekaveman
thekaveman requested changes Oct 13, 2021
View reviewed changes
thekaveman
thekaveman requested changes Oct 14, 2021
View reviewed changes
thekaveman
thekaveman approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

@machikoyasuda machikoyasuda merged commit a2cb982 into main Oct 14, 2021
@machikoyasuda machikoyasuda deleted the refactor/app-db branch October 14, 2021 19:55
@machikoyasuda machikoyasuda changed the title Refactor app.py Create Database, Verify classes Oct 14, 2021
@angela-tran angela-tran mentioned this pull request Nov 14, 2022
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thekaveman thekaveman thekaveman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
October 2021
Development

Successfully merging this pull request may close these issues.

Refactor app.py
2 participants
@machikoyasuda @thekaveman