-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Refactor build_and_tests GHA workflow before for migrating publish jobs #2313
Refactor build_and_tests GHA workflow before for migrating publish jobs #2313
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2313 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 272 272
Lines 15337 15337
=======================================
Hits 14117 14117
Misses 840 840
Partials 380 380 Continue to review full report at Codecov.
|
@bogdandrutu I separated out the naming changes and refactoring to the existing workflow into a separate PR as requested. Please take a look. |
.github/workflows/build-and-test.yml
Outdated
- name: Create Tool Binaries Archive | ||
run: tar -cvf tool-bin.tar /home/runner/go/bin |
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.
why is this needed vs the whole path?
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.
The reason I archived the file was to preserve the execute permissions on the binaries created. Specifically on the windows-msi job, there was no way to easily chmod +x
the binary once downloaded given the OS. Also, this prevents having to add permissions to the binaries everytime it is downloaded although extracting the archive is required instead.
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 am fine to always add the chmod +x instead of archive. Maybe leave also a TODO to explain this.
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.
The only issue is that I cannot do the chmod for the windows job (In PR 3/3), this was a workaround specifically for that. I can remove the archiving from tool binaries, but it is needed for the collector binaries which the windows-msi job uses.
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.
Removed the archiving from all the tool binaries, because it was unnecessary. I still need to archive the collector binary produced from cross-compile because it requires execute permissions to run in windows-msi
.
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.
It does not seem to be reflected in the code.
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 pushed my amend to the wrong Git branch, did not mean to mislead sorry. All instances of tool archive should be gone now.
9c6ad21
to
93e332c
Compare
93e332c
to
80d9700
Compare
…github.com/valyala/fasthttp v1.36.0 (open-telemetry#2313)
Which problem is this solving?
As part of issue, this pull request does some naming changes and refactoring on the already migrated build_and_test workflows jobs in preparation for migrating the remaining
windows-msi
,publish-dev
andpublish-stable
jobs in the following PRThis PR must be reviewed and merged before merging the final migration PR. The purpose of this PR is to remove changes to existing jobs from the final migration PR.
What changes are made in this PR?
cc- @alolita, @AzfaarQureshi