-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Windows] Update windows dev docs #13741
Conversation
Signed-off-by: Sotiris Nanopoulos <[email protected]>
Signed-off-by: Sotiris Nanopoulos <[email protected]>
cc @envoyproxy/windows-dev as discussed today in the Windows working group |
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.
LGTM
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.
one small comment, but LGTM
``` | ||
> On Windows the supported/recommended shell to interact with bazel is MSYS2. This means that all the bazel commands (i.e. build, test) should be executed from MSYS2. |
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.
one thought: it might be a little confusing we're setting variables in cmd
but using msys2 bash
, maybe we should add a note clarifying the two?
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 see. Do you have a suggestion in mind?
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.
Above we state that we are using cmd as an example
The paths in this document are given as examples, make sure to verify you are using the correct paths for your environment. Also note that these examples assume using a
cmd.exe
shell to set environment variables etc., be sure to do the equivalent if using a different shell.
and here we specify that MSYS2
is used only to interact with bazel.
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.
yeah, maybe its as easy as saying something on the order of "this example shows using cmd
to set env variables before entering bash
to ensure environment variables are persisted, you may use export
from within bash
to perform the equivalent steps within a bash
session", idk, we maybe don't need to be so explicit, but we've had a good few issues with build environments
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.
oh yeah thats already there i missed that, perfect
This is a docs-only change ready to merge; the CI test harness failures should be ignorable. |
/retest |
Retrying Azure Pipelines: |
* master: (83 commits) tls: Typesafe tls slots (envoyproxy#13789) docs(example): Correct URL for caching example page (envoyproxy#13810) [fuzz] Made health check fuzz more efficient (envoyproxy#13747) rtds: properly scope rtds stats (envoyproxy#13764) http: fixing a bug with IPv6 hosts (envoyproxy#13798) connection: Remember transport socket read resumption requests and replay them when re-enabling read. (envoyproxy#13772) network: adding some accessors for ALPN work. (envoyproxy#13785) docs: added a step about how to handle platform specific extensions (envoyproxy#13759) Fix identation in ip transparency code snippet (envoyproxy#13743) wasm: enable WAVM's stack unwinding feature (envoyproxy#13792) log: set route name for direct response (envoyproxy#13683) Use nghttp2 as external dependsncy in protocol_constraints_lib (envoyproxy#13763) [Windows] Update windows dev docs (envoyproxy#13741) cel: patch thread safety issue (envoyproxy#13739) Windows: Fix ssl_socket_test (envoyproxy#13264) apple dns: add fake api test suite (envoyproxy#13780) overload: scale selected timers in response to load (envoyproxy#13475) examples: Add dynamic configuration (control plane) sandbox (envoyproxy#13746) Removed exception in getResponseStatus() (envoyproxy#13314) network: add timeout for transport connect (envoyproxy#13610) ... Signed-off-by: Michael Puncel <[email protected]>
Signed-off-by: Sotiris Nanopoulos [email protected]
Update dev docs.
Risk Level: N/A
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A