-
Notifications
You must be signed in to change notification settings - Fork 33
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
EXPOSURES: DotNetBuildTaskBuilder #43
base: main
Are you sure you want to change the base?
EXPOSURES: DotNetBuildTaskBuilder #43
Conversation
Here's an example for the
will generate the following YAML snippet:
The search path, API key, and destination parameters are optional, depending on the version of NuGet.exe being used. |
I want to get feedback first before I continue down this path. If I get approval, I will continue and add tests, validation, validation tests, and documentation, if needed. |
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/Builders/DotNetBuildTaskBuilder.cs
Show resolved
Hide resolved
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/Builders/DotNetBuildTaskBuilder.cs
Outdated
Show resolved
Hide resolved
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/Builders/DotNetBuildTaskBuilder.cs
Outdated
Show resolved
Hide resolved
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/Builders/DotNetBuildTaskBuilder.cs
Outdated
Show resolved
Hide resolved
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/NuGetPushTask.cs
Outdated
Show resolved
Hide resolved
dceb79c
to
46a4dba
Compare
46a4dba
to
991a334
Compare
Thank you for the code review. I have resolved all the code review comments. |
Should I continue and create the following tests?
|
ADotNet/Models/Pipelines/GithubPipelines/DotNets/Tasks/Builders/NuGetPushTaskBuilder.cs
Show resolved
Hide resolved
@icnocop this is good. We need to place this somewhere in the Exposers layers of this library. @ElbekDeveloper feel free to chime in as well. |
@cjdutoit do you have any feedback for us on this? |
Refactored fluent methods to avoid having to prefix methods with With
@hassanhabib / @icnocop sorry, I did not see there was a wait on me for this one. I like the fluent syntax. First thoughts is that we should name the builders more closely to what we have now i.e. Should we perhaps create separate issues so be can create all these builders from the top down to match a rough structure like this? var githubPipeline = new GithubPipelineBuilder()
.Name(".Net")
.WithEvents(gitHubEventBuilders =>
{
gitHubEventBuilders.Add(new GitHubPushEventBuilder().Branches(...),
gitHubEventBuilders.Add(new GitHubPushEventBuilder().Branches(...)
})
.WithJobs(gitHubJobsBuilders =>
{
gitHubJobsBuilders.Build = new gitHubJobBuilder()
.RunsOn(BuildMachines.Windows2019)
.WithSteps(githubStepBuilders =>
{
githubStepBuilders.Add(new CheckoutTaskV2StepBuilder()),
githubStepBuilders.Add(new SetupDotNetTaskV1StepBuilder()),
githubStepBuilders.Add(new RestoreTaskStepBuilder()),
githubStepBuilders.Add(new DotNetBuildTaskStepBuilder()),
githubStepBuilders.Add(new TestTaskStepBuilder()),
githubStepBuilders.Add(new NugetDeployTaskStepBuilder()),
})
})
.Build(); |
Closes #40
I added two task builders in this PR to show a simple case and a more advanced case.
I could not remove the
--no-restore
command option from the existingDotNetBuildTask
because that would break backwards compatibility.So instead, I created a task builder which can be used with fluent syntax to specify optional parameters to the task.
For example:
will generate the following YAML snippet:
As another example:
will generate the following YAML snippet: