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: context module into a package #2697

Merged
merged 5 commits into from
Feb 20, 2025
Merged

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Feb 20, 2025

Closes #2682

This PR addresses:

Naming of the module...

  • by introducing a context folder that has a package initialization file (__init__.py) and moving the agency-specific context code into an agency module within the package

... and [naming of the] dict variables

  • by removing _context from the module-level variables

Key for the context dict

  • by using .value when constructing the dictionary

@angela-tran angela-tran requested a review from a team as a code owner February 20, 2025 01:47
@angela-tran angela-tran self-assigned this Feb 20, 2025
@github-actions github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev back-end Django views, sessions, middleware, models, migrations etc. labels Feb 20, 2025
Copy link

github-actions bot commented Feb 20, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/core/context
  __init__.py
  agency.py
  benefits/core/models
  transit.py
Project Total  

This report was generated by python-coverage-comment-action

@angela-tran angela-tran force-pushed the refactor/context-module branch from b3df575 to 007d906 Compare February 20, 2025 01:57
@angela-tran angela-tran mentioned this pull request Feb 20, 2025
3 tasks
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.

This looks great 👍

@thekaveman
Copy link
Member

I think this can close #2682 since we decided to ignore frozen data classes for now.

@angela-tran angela-tran merged commit 6aa9e2b into main Feb 20, 2025
11 checks passed
@angela-tran angela-tran deleted the refactor/context-module branch February 20, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Follow-up refinement to copy context classes and module
2 participants