Skip to content
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

Support running Octo from .net core tool-manifest #2489

Merged
merged 4 commits into from
Apr 23, 2020

Conversation

jeremyabbott
Copy link
Contributor

@jeremyabbott jeremyabbott commented Mar 31, 2020

Description

Adds support for running octo.exe as a .NET Core tool installed via a tool-manifest.

Local verification (macOS):

dotnet new fake
dotnet tool install Octopus.DotNet.Cli
#r "paket:
source https://api.nuget.org/v3/index.json
source /Users/jeremy/code/FAKE/src/app/Fake.Tools.Octo/bin/Debug
nuget Fake.DotNet.Cli
nuget Fake.IO.FileSystem
nuget Fake.Core.Target
nuget Fake.Tools.Octo 1.0.0 //"
#load ".fake/build.fsx/intellisense.fsx"
open Fake.Core
open Fake.DotNet
open Fake.Core.TargetOperators

Target.initEnvironment ()

Target.create "Octo" (fun _ ->
    
    Fake.Tools.Octo.listEnvironments
      (fun opts ->
        { opts with 
            ToolType = ToolType.CreateLocalTool()
            Server = { opts.Server with ServerUrl = "https://octopus.server"; ApiKey="API-ABCDEFG123"}
        })
)

Target.runOrDefault "Octo"

TODO

There are two obsolete calls in this module, but I'm not sure how to use the suggested replacement method.

  • New (API-)documentation for new features exist (Note: API-docs are enough, additional docs are in help/markdown)

  • unit or integration test exists (or short reasoning why it doesn't make sense)

  • boy scout rule: "leave the code behind in a better state than you found it" (fix warnings, obsolete members or code-style in the places you worked in)

  • (if new module) the module has been linked from the "Modules" menu, edit help/templates/template.cshtml, linking to the API-reference is fine.

  • (if new module) the module is in the correct namespace

  • (if new module) the module is added to Fake.sln (dotnet sln Fake.sln add src/app/Fake.*/Fake.*.fsproj)

  • Fake 5 API guideline is honored

Use "dotnet" as the executable and prepend  "dotnet-octo" to list of generated args for Octo commands
@jeremyabbott jeremyabbott marked this pull request as ready for review March 31, 2020 03:15
@jeremyabbott jeremyabbott changed the title WIP: Support running from .net core tool-manifest Support running Octo from .net core tool-manifest Mar 31, 2020
@matthid
Copy link
Member

matthid commented Apr 4, 2020

Shouldn't we use a similar API as existing modules are using (https://fake.build/dotnet-cli.html#SDK-tools-local-global-clireference) for example ReportGenerator?

@jeremyabbott
Copy link
Contributor Author

@matthid I'll take a look! Thank you for the suggestion.

@jeremyabbott jeremyabbott changed the title Support running Octo from .net core tool-manifest WIP Support running Octo from .net core tool-manifest Apr 22, 2020
@jeremyabbott
Copy link
Contributor Author

@matthid I'm sorry for taking so long to come back to this! I made the recommended changes and tested them locally. I'm on macOS:

dotnet new fake
dotnet tool install Octopus.DotNet.Cli
#r "paket:
source https://api.nuget.org/v3/index.json
source /Users/jeremy/code/FAKE/src/app/Fake.Tools.Octo/bin/Debug
nuget Fake.DotNet.Cli
nuget Fake.IO.FileSystem
nuget Fake.Core.Target
nuget Fake.Tools.Octo 1.0.0 //"
#load ".fake/build.fsx/intellisense.fsx"
open Fake.Core
open Fake.DotNet
open Fake.Core.TargetOperators

Target.initEnvironment ()

Target.create "Octo" (fun _ ->
    
    Fake.Tools.Octo.listEnvironments
      (fun opts ->
        { opts with 
            ToolType = ToolType.CreateLocalTool()
            Server = { opts.Server with ServerUrl = "https://octopus.server"; ApiKey="API-ABCDEFG123"}
        })
)

Target.runOrDefault "Octo"

@jeremyabbott jeremyabbott changed the title WIP Support running Octo from .net core tool-manifest Support running Octo from .net core tool-manifest Apr 22, 2020
```bash
dotnet new tool-manifest # if one doesn't already exist
dotnet tool install Octopus.DotNet.Cli
```
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we add here that you need to set the ToolPath and maybe link to https://fake.build/dotnet-cli.html#SDK-tools-local-global-clireference?

@matthid
Copy link
Member

matthid commented Apr 23, 2020

No need to be sorry, good things take time :)
Looks good to me in general, we could improve the docs and add some tests but that is nitpicking...

@matthid matthid merged commit 8f6030e into fsprojects:release/next Apr 23, 2020
@jeremyabbott jeremyabbott deleted the octo-tool-manifest branch April 23, 2020 14:59
@jeremyabbott
Copy link
Contributor Author

@matthid I'll follow up with those suggestions in a separate PR. Thank you again!

@matthid matthid mentioned this pull request May 4, 2020
3 tasks
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