-
Notifications
You must be signed in to change notification settings - Fork 649
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
Move all runner code except DB driver dependencies into FluentMigrator.Runner.Core #1164
Comments
@Sellec Can you explain the annoyance? This has been discussed before, but nobody has really provided us a real reason for their annoyance other than extra files in the directory. As previously discussed, adding all the runners only adds ~451Kb right now, which, if you think of disk space as like toilet paper, is less than a single sheet of 1000 sheet toilet paper on modern SSD hard drives. There has to be a good reason for this annoyance other than "it's annoying", because we're all doing this project on our free time.
FYI: AddFluentMigratorCore adds all these assemblies you find annoying. If you want a practical solution to this problem, it would mean creating separate projects for every dependency. This is what EntityFrameworkCore does. I guess we could go down that route, but it's a lot more work and the only obvious benefit is ~451Kb? I'd rather fix the backlog of bugs in MySQL than fix this. |
I mean, that's one way to view the analogy. On the other hand, if you want to read about macrophages and the role they play in triggering your body's immunological response, you don't just rip out the chapter on macrophages from a book titled, Immunology. There's a lot of different biological triggers in such a book. From a practical sense, how would you propose FluentMigrator is packaged differently? Keep in mind that FluentMigrator.Runner is used by FluentMigrator.MSBuild, FluentMigrator.DotNet.Cli, FluentMigrator.Console, and FluentMigrator nuget packages. Would you create a FluentMigrator.MSBuild.SqlServer, FluentMigrator.MSBuild.OracleManagedClient, FluentMigrator.MSBuild.DB2, etc and so on for FluentMigrator.DotNet.Cli as well? Note that this transformation would necessarily have to guarantee the right .deps.json files are shipped for the "External Runners" (FluentMigrator.Console, FluentMigrator.MSBuild, FluentMigrator.DotNet.Cli). I'm not saying this is how it must be to achieve your wish, but I suppose I can just repeat the question you asked here back to you: How would you structure things so you only use SqlServer Runner? Note: I'm not being sarcastic here - I am genuinely curious if there is a configuration of packages that works best for everyone, and adds economic value to end users (e.g., fewer bugs). If there is, let's do that. |
I must agree with Sellec here. Having all these dependencies just to get AddFluentMigratorCore is a bit ugly. Having to add provider specific dependencies is not a nuisance to me. It feels clean. But YMMV, and maybe just having FluentMigrator.Runner become a pure meta package could be the way to go. |
It's not just to get AddFluentMigratorCore. It's to generate the correct deps.json file, too. That's what makes this tricky and why we're not excited to "make things cleaner" at the expense of a likely avalanche of confusion. People still don't really understand how to package core apps, and it's debatable whether the maintainers of this project should be the ones to burden the teaching load. Thoughts? |
I agree that additional complexity should not be added, but as it is now FluentMigrator.Runner.Core is already much more than an abstraction library. It already has the dependencies to DependecyInjection etc. so everything that has to do with "running" could live there. I could see a reason to have a slim abstraction library used for creating new runner types, but that's not where the code is headed and it's not necessarily worthwhile. In other words, almost everything that's in FluentMigrator.Runner could move into FluentMigrator.Runner.Core. FluentMigrator.MSBuild, FluentMigrator.DotNet.Cli, FluentMigrator.Console, and FluentMigrator would continue to work as before, and the seemingly unnatural (please correct me if I'm wrong) distinction between FluentMigrator.Runner and FluentMigrator.Runner.Core would disappear. |
Unless I am mistaken, You didn't address the core problem of bootstrapping the runners so that they "just work". That is what I meant by saying generating a correct deps.json file is key. It's difficult enough as-is. I have spent a lot of time trying to simplify this, and have come to the conclusion that it's a bad business for open source developers to be in to write "linkers" and "dll linking code". It's better to let the compiler do it for you. That's what the .NET Core compiler does when it generates deps.json. |
I think that should be a non-issue. I will (try to) create a PR to prove my point. Maybe I will see the light/darkness you're talking about. |
@gliljas I started work this weekend on supporting .netcoreapp3.1 TFM which may blow up your PR. I'll try to merge in the changes to your PR. |
I commented on @gliljas PR here: #1198 (comment) Long story short, I have changed my mind and think this is a good idea. I am pretty exhausted from working a lot lately, but I am close to shipping FluentMigrator 3.3 and 3.3.1 milestones - they're basically in the bag with all tests passing, but just need energy to finish some touches/cleanup a few things. |
New PR at #1600 |
Its is very annoying sight becouse FluentMigrator.Runner includes lots of dependent packages.
![Clip2net_200210210826](https://user-images.githubusercontent.com/3831263/74176867-9dae7080-4c49-11ea-9d59-41971f70b105.png)
But FluentMigrator.Runner.Core does not provides needed methods like FluentMigratorServiceCollectionExtensions.AddFluentMigratorCore
The text was updated successfully, but these errors were encountered: