Enable config for release artifacts #1063
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1063 +/- ##
==========================================
+ Coverage 70.13% 72.06% +1.93%
==========================================
Files 52 50 -2
Lines 6127 6126 -1
==========================================
+ Hits 4297 4415 +118
+ Misses 1630 1503 -127
- Partials 200 208 +8
Continue to review full report at Codecov.
|
bin/init.sh
Outdated
| @@ -9,7 +19,7 @@ if [ $PDIR != "$GOPATH/src/istio.io/pilot" ]; then | |||
| fi | |||
|
|
|||
| # Building and testing with Bazel | |||
| bazel build //... | |||
| bazel build ${bazel_build_args[@]} //... | |||
There was a problem hiding this comment.
If you really need to customize bazel build, then I suggest remove this line from this script.
There was a problem hiding this comment.
Removing and building where ?
There was a problem hiding this comment.
I am not sure what you meant here.
bin/push-docker
Outdated
|
|
||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| -tag) tags="$2"; shift ;; | ||
| -hub) hubs="$2"; shift ;; | ||
| -config) bazel_build_args+=(--config="$2"); shift ;; |
There was a problem hiding this comment.
can we call it bazelconfig or something more specific than "config"?
bin/upload-istioctl
Outdated
| @@ -7,19 +7,22 @@ | |||
| set -o errexit | |||
| set -o nounset | |||
| set -o pipefail | |||
| set -ex | |||
There was a problem hiding this comment.
why? that's same as options above
There was a problem hiding this comment.
I ll remove it, this was just for debugging to print the commands.
tools/bazel.rc
Outdated
| @@ -2,5 +2,8 @@ | |||
| build --verbose_failures | |||
| test --test_output=errors | |||
|
|
|||
| # Release Build | |||
| build:release -c opt | |||
There was a problem hiding this comment.
does -c opt even have any effect on golang?
There was a problem hiding this comment.
From @ldemailly it does !
There was a problem hiding this comment.
what changes in the go binary?
There was a problem hiding this comment.
the 2x benefits I measured is with envoy (c++) but I expect debug to be -gcflags="-N -l" and opt to be without those and thus make a difference, not sure the bazel go rules do do that (we should check, but even if they don't today, having 2 or more config will be useful in the future)
There was a problem hiding this comment.
yes, envoy is special. let's first check if there's any difference with rules_go. if not, then we can't even use bazel to accomplish optimized build.
There was a problem hiding this comment.
I asked in bazelbuild/rules_go#741 but we should use this config seb is adding anyway, to change the default/compiled in behavior of kube-inject / to select the optimized sidecar images for instance
There was a problem hiding this comment.
I wish I had known that before starting.
bin/upload-istioctl
Outdated
| case ${arg} in | ||
| c) INIT_ARGS+=(-config ${OPTARG});; |
There was a problem hiding this comment.
can we avoid mysterious short flags and use same flag as above?
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Closing this as this is does not have any impact on the artifacts. |
|
it will in next bazel: see bazelbuild/rules_go#743 |
|
keep the PR open until rules_go releases the support. |
|
or we could just get it in and it'll get benefits automatically once bazel side changes - whichever works for you |
|
closing this as I just force pushed to this branch and lost the code for this :( |
|
the bazel change is in, can we at least keep this for early part of 0.3 |
|
we can but I lost the code, so I have to it all over again. |
|
code is here no ? https://patch-diff.githubusercontent.com/raw/istio/pilot/pull/1063.patch |
|
or use the reflog to recover the code ? |
|
The code that you pointed out refer to another PR. Tell me more about
reflog.
…On Aug 28, 2017 4:19 PM, "Laurent Demailly" ***@***.***> wrote:
or use the reflog to recover the code ?
(but we can close this if you prefer - and just leave the issue)
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#1063 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQ80ZzvtH7mYQEhahuBcgua5xjhe1fi0ks5sc0r5gaJpZM4O78g0>
.
|
|
you can do "git reflog" and see your local history and go back to any point in that history
|
|
it's gone! |
No description provided.