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

Refactor/database #1220

Merged
merged 28 commits into from
Jul 6, 2024
Merged

Refactor/database #1220

merged 28 commits into from
Jul 6, 2024

Conversation

vitormpp
Copy link
Member

@vitormpp vitormpp commented Apr 3, 2024

Closes #835

  • Enhanced encapsulation for persistent and non-persistent session logic within database classes.
  • Made the application database class abstract and refactored database classes accordingly.
  • Refactored providers for improved structure and clarity.

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@bdmendes bdmendes added this to the July 2024 Release milestone May 22, 2024
@bdmendes bdmendes self-requested a review May 29, 2024 13:25
@bdmendes
Copy link
Collaborator

bdmendes commented Jun 2, 2024

@vitormpp is this ready?

@vitormpp vitormpp force-pushed the refactor/data-base branch from fef984a to 728982d Compare June 5, 2024 13:30
@DGoiana DGoiana requested review from bdmendes and rubuy-74 June 5, 2024 13:30
@vitormpp vitormpp marked this pull request as ready for review June 5, 2024 14:13
@bartekpacia
Copy link
Collaborator

@vitormpp Could you rebase/merge with the master branch? I could then try to help you debug it.

@vitormpp vitormpp changed the title Refactor/data base Refactor/database Jul 3, 2024
@vitormpp vitormpp force-pushed the refactor/data-base branch from 193736e to 82d6052 Compare July 3, 2024 17:12
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 18.27957% with 76 lines in your changes missing coverage. Please review.

Project coverage is 17%. Comparing base (1809d46) to head (9cb9871).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1220   +/-   ##
=======================================
+ Coverage       17%     17%   +1%     
=======================================
  Files          229     229           
  Lines         6986    6992    +6     
=======================================
+ Hits          1167    1187   +20     
+ Misses        5819    5805   -14     

@DGoiana DGoiana requested a review from a team July 3, 2024 21:23
Copy link
Member

@thePeras thePeras left a comment

Choose a reason for hiding this comment

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

nice work! ✅ 🛫

@DGoiana
Copy link
Collaborator

DGoiana commented Jul 4, 2024

@vitormpp can you resolve this and merge it?

@thePeras thePeras requested a review from bdmendes July 5, 2024 08:03
@vitormpp vitormpp dismissed bdmendes’s stale review July 5, 2024 23:44

The review has been taken into account. No further action is required at this time.

@DGoiana
Copy link
Collaborator

DGoiana commented Jul 6, 2024

The code looks very clean and well-written. Wonderful abstraction here. Nice job!

@DGoiana DGoiana merged commit 0ed5a70 into develop Jul 6, 2024
6 checks passed
@DGoiana DGoiana deleted the refactor/data-base branch July 6, 2024 14:50
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.

Encapsulate session not persistent logic (do not save data) in the db class
5 participants