search

toms3t / Propalyzer

Web app that helps investors evaluate residential investment property opportunities
MIT License
67 stars 28 forks source link

Enhancement/refactoring #95

Closed Nitsujed closed 5 years ago

Nitsujed commented 5 years ago

94 The cleans up the view file to take out all the logic that was being performed and much more simplified.

I duplicated some of the property logic to support a new file called context_data.py. I think we should use this class to handle the calculations and non-API necessary work. We can pass data back and forth between the deeper pages without having to unnecessarily call APIs again.

toms3t commented 5 years ago

@Nitsujed Awesome! thanks for the refactor. I'm introducing CI/CD for this app through Azure Pipelines such that any update to the master branch will trigger a new build and new release. I should have that done today. Next up, will get your PR reviewed ASAP.

Nitsujed commented 5 years ago

@toms3t I branched from master which didn't have the pdf pulled in yet. So if you are good with this merge, I will pull the changes into the pdf branch and submit a new request once it's cleaned up. Also, I think I still want to do more cleanup, so I will prob leave this branch open for continued repairs.

toms3t commented 5 years ago

@Nitsujed sounds good on the PDF merge. You can ignore that PR that just came through from the azure-pipelines branch, just testing things out.

toms3t commented 5 years ago

@Nitsujed - got CI/CD working in Azure Pipelines. Also, pulled your code in and ran through the typical use cases with no problems. Let's merge this with your PDF feature into a single PR like we discussed. Also, can you update 'test_property.py' so that we have updated tests based on the new code? The tests will have to pass before the merge into master. Thanks for your work on this!

Nitsujed commented 5 years ago

As discussed above, I am going to initiate a new pull request.