-
Notifications
You must be signed in to change notification settings - Fork 773
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
[repo] CI tweaks and improvements #5651
[repo] CI tweaks and improvements #5651
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5651 +/- ##
==========================================
+ Coverage 83.38% 85.78% +2.39%
==========================================
Files 297 254 -43
Lines 12531 11036 -1495
==========================================
- Hits 10449 9467 -982
+ Misses 2082 1569 -513
Flags with carried forward coverage won't be shown. Click here to find out more. |
carryforward: true | ||
paths: | ||
- src | ||
|
||
unittests-Instrumentation-Experimental: | ||
unittests-UnstableCoreLibraries-Experimental: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there still a concept called "core libraries" after instrumentation libs moved away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's two tags:
<MinVerTagPrefix>core-</MinVerTagPrefix>
<MinVerTagPrefix>coreunstable-</MinVerTagPrefix>
But I get your point, we could probably just use stable-
and unstable-
since everything is now "core" 🤣 Follow-up!
- name: dotnet pack OpenTelemetry.proj | ||
run: dotnet pack OpenTelemetry.proj --configuration Release | ||
- name: dotnet pack | ||
run: dotnet pack ./build/OpenTelemetry.proj --configuration Release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this file move affect existing docs? e.g. https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/CONTRIBUTING.md#pull-requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. I searched the repo for all occurrences of OpenTelemetry.proj
to make sure I wasn't breaking anything. What I will do (follow-up) is mention in that guide new files\projects should be added to the solution.
|
||
- name: dotnet format | ||
run: dotnet format OpenTelemetry.sln --no-restore --verify-no-changes | ||
run: dotnet format OpenTelemetry.sln --no-restore --verify-no-changes # Note: .proj files are currently not supported by dotnet format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would run dotnet format over OpenTelemetry.proj
but that isn't supported currently. This means projects not added to the solution won't have this linting performed. I tried some powershell to find all the .csproj
files and manually call dotnet format over them but I canceled my test run after 17+ minutes. IMO having it run quickly and potentially missing things is the lesser evil than covering everything and it taking forever to run.
Changes
OpenTelemetry.proj
instead ofOpenTelemetry.sln
for code buildsOpenTelemetry.sln
if it is changedDetails
I noticed a couple gaps in our CI:
For 1 there is now a build for solution if it is changed.
For 2 we now use
OpenTelemetry.proj
for builds instead of the solution. The project is more dynamic and picks up all.csproj
files automatically.Merge requirement checklist