Create the Asset Database #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidtaylorhq
requested changes
Dec 4, 2019
There was a problem hiding this comment.
Skimmed through it and left a few comments. In general it looks good to me. Two general suggestions:
-
Can we get shrink down the number of migration files? They've clearly grown as development has happened, but it's quite messy now. You should be able to simply delete them all, then
makemigrationsagain to generate a single file. That might break your local databases, but you can just throw them away and generate sample data again -
There are no tests. Ultimately it's up to you, but I would recommend adding at least a few for the basic functionality. They have saved us a few times with RIGS!
|
One more thing - this branch needs re-basing onto master, or have master merged into it. The latter may be easiest. |
Again. Again again.
Surprisingly, not my fault, it was @mattysmith22
I blame @mattysmith22 again ...
TODO: Make the colour defined by the status as opposed to hardcoding it...
Red is for broken/lost - things we should have but don't Yellow is for sold/scrapped - things we had but don't have on purpose Blue is not built yet because... I don't have a reason for that one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR opened for Heroku deployment purposes, it is not quite ready yet!