Skip to content

CommandLine plumbing infrastructure#652

Merged
dylan-smith merged 9 commits intomainfrom
commandline-plumbing-infrastructure
Sep 23, 2022
Merged

CommandLine plumbing infrastructure#652
dylan-smith merged 9 commits intomainfrom
commandline-plumbing-infrastructure

Conversation

@ArinGhazarian
Copy link
Contributor

@ArinGhazarian ArinGhazarian commented Sep 21, 2022

Related to #639

This is all the required infrastructure changes in form of some extension method to do the plumbing for the new CommandLine. Here is a sample branch that applies these changes to bbs2gh.
I went ahead and simplified it even more by creating an extension to add all commands to the rootCommand. Here is how it's supposed to be used:

public static async Task<int> Main(string[] args) // note that Main now returns a Task<int> instead of Task
{
    Logger.LogDebug("Execution Started");
 
    var serviceCollection = new ServiceCollection();
    ... // service registrations
    var serviceProvider = serviceCollection.BuildServiceProvider();

    var rootCommand = new RootCommand("Automate end-to-end Bitbucket Server to GitHub migrations.")
          .AddCommands(serviceProvider);

    ...
}
  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)

{
var commandArgsType = commandType.BaseType.GetGenericArguments()[0];
var commandArgs = ctx.BindArgs(command, commandArgsType);
var handler = commandType.GetMethod("BuildHandler").Invoke(command, new[] { commandArgs, serviceProvider });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to check if handler implements ICommandHander and throw a meaningful exception? It is going to throw a NullReferenceException if GetMethod returns null in case it cannot find BuildHandler method anyways.

@github-actions
Copy link

github-actions bot commented Sep 21, 2022

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit bdd4a62.

♻️ This comment has been updated with latest results.

@ArinGhazarian ArinGhazarian marked this pull request as ready for review September 21, 2022 01:55
@ArinGhazarian ArinGhazarian changed the title Commandline plumbing infrastructure CommandLine plumbing infrastructure Sep 21, 2022
@github-actions
Copy link

github-actions bot commented Sep 21, 2022

Integration Test Results

4 tests  ±0   4 ✔️ ±0   14m 46s ⏱️ - 1m 43s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ±0 

Results for commit ecc9bdf. ± Comparison against base commit c752c8e.

♻️ This comment has been updated with latest results.

@dylan-smith dylan-smith enabled auto-merge (squash) September 23, 2022 19:55
@dylan-smith dylan-smith merged commit 904869c into main Sep 23, 2022
@dylan-smith dylan-smith deleted the commandline-plumbing-infrastructure branch September 23, 2022 19:59
@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
Octoshift 89% 75% 854
bbs2gh 73% 65% 261
ado2gh 88% 83% 578
gei 81% 87% 429
Summary 85% (5378 / 6304) 78% (1162 / 1490) 2122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants