Feature/di

[arturcic]

Refactored code to prepare the addition of DI to improve the modularity/testability.

Some of the changes introduced:

• Introduce GitVersionApplication
• Introduce several interfaces instead of creating class instances directly (IArgumentParser, IConfigFileLocator)
• Replaced the old Logger static class with ILog interface with 2 appenders - ConsoleAppender and FileAppender. Updated the code to use the new ILog

This changes will allow gradually to introduce a DI (Autofac) so that we can later on split in modules some of the functionalities we currently support as addins/plugins call them whatever you want.

The GitVersion itself will just take as input the arguments and return as output the GitVersion information that later can be consumed by these plugins.

Example of plugins could be:

• AssemblyInfo updater
• BuildServer integration
• In the future we'll be able to decouple the Git metadata reading from the version computation in separate module and so on

[asbjornu]

I whole-heartedly condone and applaud the changes you're doing here, @arturcic! 🎉 👏 ❤️

I just have a few questions and comments made inline. I also wonder whether with all the refactoring you're doing here, there might be an opportunity to move code from GitVersionExe and GitVersionTask into GitVersionCore in order to tackle #883?

[arturcic]

I whole-heartedly condone and applaud the changes you're doing here, @arturcic! 🎉 👏 ❤️

I just have a few questions and comments made inline. I also wonder whether with all the refactoring you're doing here, there might be an opportunity to move code from GitVersionExe and GitVersionCore into GitVersionCore in order to tackle #883?

I think you meant GitVersionExe and GitVersionTask into GitVersionCore. That makes sense, but as part of a different refactoring.

[asbjornu]

I think this looks great. The only thing I oppose is your wish to use Autofac as the DI Container. I wouldn't mind that much if the DI Container integration was made plugin-based so different containers could be used, but if we need to add a dependency to a DI Container, I think Microsoft's own would be the most non-controversial choice. Thoughts?

[asbjornu]

I think you meant GitVersionExe and GitVersionTask into GitVersionCore.

Correct.

That makes sense, but as part of a different refactoring.

Ok.

[arturcic]

I think this looks great. The only thing I oppose is your wish to use Autofac as the DI Container. I wouldn't mind that much if the DI Container integration was made plugin-based so different containers could be used, but if we need to add a dependency to a DI Container, I think Microsoft's own would be the most non-controversial choice. Thoughts?

To be honest I was thinking of Autofac because I have used it before, and also Cake-Build team is using it. I think we can use the DI from the .net core generic host https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/generic-host?view=aspnetcore-3.0. Or you mean something else?

[asbjornu]

Are these changes so massive that they warrant a new major bump?

[arturcic]

It's more a cleanup/refactor than big changes. There are no major breaking changes. There is only one important change, but backward compatible for now. The verbosity flag is using both LogLevel and Verbosity enums, but at some point we'll remove the loglevel enum and keep verbosity. The old Verbosity became LogLevel

[arturcic]

This change was made to avoid the confusion between verbosity and loglevel. Now the LogLevel is used for the type of the log - Warn, Info and so on, and Verbosity for how verbose the log output is.

[arturcic]

and I think we can already merge this, I have a pending draft PR based on this PR that introduce the DI container that you suggested, and I will need some feedback on that one

[asbjornu]

Excellent. Great work!