Update build telemetry to use the local IceRpc packages #4040
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pepone
commented
Sep 9, 2024
| @@ -66,8 +65,25 @@ | |||
|
|
|||
| BuildTelemetry buildTelemetry = idl.ToLower() switch | |||
| { | |||
| "slice" => new BuildTelemetry.Slice(new SliceTelemetryData(version, compilationHash, containsSlice1, containsSlice2, sourceFileCount, referenceFileCount)), | |||
| "protobuf" => new BuildTelemetry.Protobuf(new ProtobufTelemetryData(version, compilationHash, sourceFileCount)), | |||
There was a problem hiding this comment.
Reworked the code to use the generated constructor, and remove the other constructor that hardcodes the defaults
externl
approved these changes
Sep 9, 2024
bernardnormier
approved these changes
Sep 9, 2024
| @@ -20,12 +20,12 @@ | |||
| <SliceCompilerPath | |||
| Condition="'$(Configuration)' == 'Release'" | |||
| >$(MSBuildThisFileDirectory)../slicec-cs/target/release</SliceCompilerPath> | |||
| <IceRpcBuildTelemetryScriptPath | |||
| >$(MSBuildThisFileDirectory)../IceRpc.BuildTelemetry.Reporter/bin/$(Configuration)/net8.0/</IceRpcBuildTelemetryScriptPath> | |||
| <!-- Build telemetry is not supported with source builds --> | |||
There was a problem hiding this comment.
I would update the comment to:
Disable build telemetry when using a source build of slicec-cs
InsertCreativityHere
approved these changes
Sep 9, 2024
Comment on lines
+19
to
+22
| /// <summary> | ||
| /// Gets or sets the number of source files in the Slice compilation. | ||
| /// </summary> | ||
| public int SourceFileCount { get; set; } |
There was a problem hiding this comment.
This should say protobuf instead of Slice compilation.
Also, I'm not sure whether making a distinction of "source" files is meaningful/correct with Protobuf,
unlike in Slice.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates
IceRpc.BuildTelemetry.Reporterpackage to use the local IceRPC packages, previously it was build against the published NuGet packages.It also splits the
TelemetryTaskin twoBuildTelemetryTaskclasses one inIceRpc.Slice.Toolsand another inIceRpc.Protobuf.Tools.Fixes #4025