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

Code Refactor #9142

Open
wants to merge 19 commits into
base: development
Choose a base branch
from
Open

Code Refactor #9142

wants to merge 19 commits into from

Conversation

T2703
Copy link

@T2703 T2703 commented Dec 8, 2024

Short description of what this resolves:

These changes help refactor the code making it easier to maintain for the future and consistency for the software's architecture. With these changes it will help ensure that the code base is easier to understand and helps remove unnecessary lines of code that don't need to be there and can help speed up performance and can reduce the amount of bugs. These newer changes also help separate domain model and application logic.

More clarification on the changes:
In the event model the starts_at and ends_at date time field have been refactored to a TSTZRANGE data type since the date times were heavily connected it would be better to simplify them to a single field on the model and the new data type would help reduce the boilerplate code.

The domain driven bounded contexts on this project is not clarified enough and thus we created context modules such as event, financial, communication, and form. This will help maintain the DDD for the codebase.

Another change for the SOLID principles on this project. The new changes also help separate domain model and application logic by breaking a huge method into smaller sub methods making it easier to maintain and understand it also helps keeps simplicity.

Checklist

  • [ ✓] I have read the Contribution & Best practices Guide and my PR follows them.
  • [✓ ] My branch is up-to-date with the Upstream development branch.
  • [✓ ] The unit tests pass locally with my changes
  • [ ✓] I have added tests that prove my fix is effective or that my feature works
  • [✓ ] I have added necessary documentation (if appropriate)
  • [✓ ] All the functions created/modified in this PR contain relevant docstrings.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

The pull request #9142 has too many files changed.

We can only review pull requests with up to 300 changed files, and this pull request has 302.

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.

5 participants