Update build.cake #80
Conversation
Refactor Cake script for optimization and clarity - Consolidated common settings to reduce redundancy and improve maintainability - Combined task criteria for better readability - Consolidated NuGet push tasks into one task for simplicity - Made minor formatting improvements for consistency These changes enhance the efficiency, readability, and maintainability of the Cake script.
what do you mean here? |
| var testResultsDir = artifactsDir.Combine("test-results"); | ||
| var psModuleDir = artifactsDir.Combine("Grok"); | ||
| var testCoverageOutputFilePath = testResultsDir.CombineWithFilePath("OpenCover.xml"); | ||
|
|
||
| var projectFileMain = File("./src/Grok.Net/Grok.Net.csproj"); | ||
| var projectFilePowerShell = File("./src/Grok.Net.PowerShell/Grok.Net.PowerShell.csproj"); | ||
|
|
||
| var commonSettings = new DotNetBuildSettings |
There was a problem hiding this comment.
It'd be better to rename it to buildSettings to make it consistent with testSettings.
But what's the reason to make it a global variable instead of keeping it in the Build task. Only the Build task uses it!
There is testSettings which is the same but you didn't move it...
Could you explain pls
There was a problem hiding this comment.
Thanks for your feedback, Marusyk. I've updated the variable name to buildSettings to maintain consistency with testSettings, as you suggested. Regarding your question about making it a global variable versus keeping it within the Build task, I initially considered potential future use of commonSettings across multiple tasks, hence the decision to define it globally. However, I see your point about encapsulation and clarity. Upon reflection, I've moved both buildSettings and testSettings into their respective tasks for better scoping and organization. Let me know if you have any further suggestions or concerns.
| .WithCriteria(!string.IsNullOrWhiteSpace(coverallsRepoToken)) | ||
| .WithCriteria((context) => !BuildSystem.IsPullRequest) | ||
| .WithCriteria((context) => !BuildSystem.IsLocalBuild) | ||
| .WithCriteria(context => FileExists(testCoverageOutputFilePath) && |
There was a problem hiding this comment.
Could you please explain what is the benefit of these changes?
How does it make better readability?
| @@ -128,7 +127,7 @@ Task("PsModulePack") | |||
|
|
|||
| Task("PsModulePush") | |||
| .IsDependentOn("PsModulePack") | |||
| .WithCriteria(!string.IsNullOrWhiteSpace(psNugetApiKey)) | |||
| .WithCriteria(context => !string.IsNullOrWhiteSpace(psNugetApiKey)) | |||
| var testResultsDir = artifactsDir.Combine("test-results"); | ||
| var psModuleDir = artifactsDir.Combine("Grok"); | ||
| var testCoverageOutputFilePath = testResultsDir.CombineWithFilePath("OpenCover.xml"); | ||
|
|
||
| var projectFileMain = File("./src/Grok.Net/Grok.Net.csproj"); | ||
| var projectFilePowerShell = File("./src/Grok.Net.PowerShell/Grok.Net.PowerShell.csproj"); | ||
|
|
||
| var commonSettings = new DotNetBuildSettings |
Refactor Cake script for optimization and clarity - Consolidated common settings to reduce redundancy and improve maintainability - Combined task criteria for better readability - Consolidated NuGet push tasks into one task for simplicity - Made minor formatting improvements for consistency These changes enhance the efficiency, readability, and maintainability of the Cake script.