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

Encrypt databases #1097

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Encrypt databases #1097

wants to merge 1 commit into from

Conversation

S1210
Copy link

@S1210 S1210 commented May 29, 2023

DB encryption #1058

@EngrTaofeek
Copy link
Collaborator

@S1210 Thanks on your first PR, here are few notes:
1 The PR contains too many changes which makes it very tough to review. For instance there are some parts I would like to approve and some I won't.
2 Let each pull request address an issue and not multiple except if they are really intertwined and can't be done seperately.
3. This PR failed CI/CD test which is a requirement for pull request to be merged
4. Refactor should be done to improve code quality and not just renaming files or doing same thing in a different way with no direct improvement.
5. Library updates in grade file should also be tested very well as some library versions are not compatible with other versions

@S1210 S1210 force-pushed the encrypt_database branch from 6d0221d to 6f6bc74 Compare May 31, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@S1210 @EngrTaofeek