Added documentation for flattening migrations #47
Conversation
…bout the process of flattening migrations.
Codecov Report
@@ Coverage Diff @@
## main #47 +/- ##
=======================================
Coverage 28.29% 28.29%
=======================================
Files 60 60
Lines 880 880
Branches 201 201
=======================================
Hits 249 249
Misses 631 631 Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| public bool ValidateFlattenedShouldRun(string id) |
There was a problem hiding this comment.
Nitpick - alphabetization of methods
| * In the `FlattenedMigration` class, add a new const for the ID of this migration. It should, when sorted as strings, be next in line from the previous flattened migration. Additionally, add it to the static list that contains all the migration ID consts. | ||
| * Open the `Designer.cs` file of the flattened migration, and update the name in the `[Migration("")]` attribute to use the const you just added. | ||
| * Rename the files themselves to match what you put in the `[Migration("")]` attribute. | ||
| 7. Manually update the flattened migration to call `ValidateFlattenedShouldRun` passing in the ID of the migration you are working off of. This will check if, based on the sort order of the other flattened migrations, this flattened migration has not run on a fresh install. If that method call returns false, you should abort running the flattened migration, as it means this is not a fresh install. |
There was a problem hiding this comment.
Can some subset of this comment be added as a comment above the method in that base class?
…base class, per feedback from PR.
| @@ -0,0 +1,88 @@ | |||
| using Microsoft.EntityFrameworkCore; | |||
There was a problem hiding this comment.
Awesome! What are your thoughts on migrating this to the Data.SqlServer package? I set it up this morning being I know we are going to need it. One of our clients could actually use this soon as it is on their backlog. https://github.com/AndcultureCode/AndcultureCode.CSharp.Data.SqlServer
There was a problem hiding this comment.
Perhaps projects could extend this, being required to override specific aspects and 'context' be a generic?
There was a problem hiding this comment.
Sorry for switching gears on your in terms of OSS packages - if you don't have time, we can just create a github issue do to it later on
There was a problem hiding this comment.
I should be able to migrate this over on Friday. Once I do that, I'll also update this repo to use that package (I'll also address the master vs main issue when I migrate this over).
| @@ -0,0 +1,17 @@ | |||
| # Flattening Migrations | |||
|
|
|||
| 1. Checkout greatest common denominator brach -- typically `master`. | |||
There was a problem hiding this comment.
Suggestion: Switch 'master' to 'main'
|
@Stefanie899 sorry, i just noticed i didn't "submit" my review so my comments from early this morning were sitting there pending. |
|
@all-contributors please add @Stefanie899 for code, reviews and documentation |
|
I've put up a pull request to add @Stefanie899! 🎉 |
Added documentation for flattening migrations, as this is something that is rarely done but extremely helpful for larger project.
Testing this would be cumbersome with little benefit in my opinion (the important portion of code, getting the index of the flattened migration in the overall list of migrations, does not lend itself well to testing), so I opted not to write automated tests. Therefore, automated test coverage may decrease from this PR, but it should be minor.