Skip building HotROD for all platforms for pull requests #5765
Conversation
Signed-off-by: Manoramsharma <84619980+Manoramsharma@users.noreply.github.com>
|
@yurishkuro I am bit confused about adding the integration test for local build image and the support for debug image using |
Signed-off-by: Manoramsharma <84619980+Manoramsharma@users.noreply.github.com>
Done. Thanks for the review 😄 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5765 +/- ##
==========================================
- Coverage 96.66% 96.65% -0.02%
==========================================
Files 341 341
Lines 16451 16451
==========================================
- Hits 15902 15900 -2
- Misses 361 362 +1
- Partials 188 189 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Manoramsharma <84619980+Manoramsharma@users.noreply.github.com>
|
@yurishkuro I have made changes for some failing checks too that I witnessed from the previous commit. |
yurishkuro
changed the title
scripts/hotrod-integration-test.sh
Outdated
| # Extract the architecture from the platform string | ||
| arch=${platform##*/} # Remove everything before the last slash | ||
| make "build-all-in-one" GOOS=linux GOARCH="${arch}" | ||
| make "build-all-in-one" GOOS="${os}" GOARCH="${arch}" |
There was a problem hiding this comment.
This is actually incorrect. This script is for building & testing hotrod. You are not actually reducing platforms on which the hotrod is build (L40-43). On the other hand, all-in-one does not need to be built for other platforms at all, only once for the current platform "$(go env GOOS)/$(go env GOARCH)", because we only use it for e2e test and then discard.
There was a problem hiding this comment.
Okay so I should leave build-all-in-one with the single build just for linux and should bring back the implementation for hotrod, something like this >
# Build for each platform for platform in $(echo "$PLATFORMS" | tr ',' ' '); do os=${platform%/*} # Extract OS (part before the slash) arch=${platform##*/} # Extract architecture (part after the slash) make build-examples GOOS="${os}" GOARCH="${arch}" done
There was a problem hiding this comment.
Yes the loop is for hotrod, allinone only needs to be built locally
yurishkuro
added
the
changelog:ci
|
Thanks! |
…ng#5765) ## Which problem is this PR solving? - Resolves jaegertracing#5743 ## Description of the changes - Similar to the functionality we have for all-in-one image I tried to achieve same build configuration supported by corresponding script file. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Manoramsharma <84619980+Manoramsharma@users.noreply.github.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <github@ysh.us>
Which problem is this PR solving?
Description of the changes
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test