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

TP2000-1404: Reorganise measures and common code #1266

Merged
merged 12 commits into from
Jul 26, 2024
Merged

Conversation

eadpearce
Copy link
Contributor

@eadpearce eadpearce commented Jul 17, 2024

TP2000-1404: Reorganise measures and common code

Why

  • measures/forms.py and measures/views.py were getting very long and hard to navigate (1000+ lines)

What

  • Splits up measures forms and views and common views code into separate files inside directories
  • Adds imports to __init__ files so we can still use the same import paths - measures.forms, measures.views, common.views

@eadpearce eadpearce requested a review from a team as a code owner July 17, 2024 11:24
@@ -0,0 +1,3 @@
from .base import *
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered importing every function explicitly but decided against it since it would just be a burden on devs making changes to those files in future having to remember to explicitly import new functions in here (I know I would definitely forget and then wonder why I'm getting an error)

@@ -0,0 +1,6 @@
from .base import *
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same again here. import * in this particular context is more future-proof than importing everything explicitly

Copy link
Collaborator

@paulpepper-trade paulpepper-trade left a comment

Choose a reason for hiding this comment

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

Seems sensible!
Is this purely a reorganisation without any changes that could introduce side effects (there're a lot of changes making it difficult to see whether there are any)?

@eadpearce
Copy link
Contributor Author

@paulpepper-trade Nope. No code changes. Just moving things around and adding the init imports

@eadpearce eadpearce merged commit c2cf34b into master Jul 26, 2024
8 checks passed
@eadpearce eadpearce deleted the TP2000-1404-refactor branch July 26, 2024 15:02
emileswarts pushed a commit that referenced this pull request Jul 29, 2024
* Split up common views

* Split up measures views and forms code

* Add imports to module __init__ files

* Fix test

* Remove empty files

* Tidy up common views imports

* Revert imports

* Revert import

* Revert comment

* Fix import
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.

2 participants