multi: take over multiple small stale PRs #4790
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To make it possible to communicate over gRPC with an instance running in Docker we need to expose the correct RPC interface.
guggero
added
code health
This was referenced Nov 21, 2020
carlaKC
approved these changes
Nov 23, 2020
| @@ -1836,8 +1838,6 @@ out: | |||
| } | |||
| } | |||
|
|
|||
| p.wg.Done() | |||
There was a problem hiding this comment.
Slight behaviour change here:
- Previously:
wg.Donethen disconnect - Now: disconnect then
wg.Done
Seems like this new ordering is the right way, but perhaps just check that nothing strange happens with the change?
There was a problem hiding this comment.
Ah, good catch! I looked at the initial commit message that added the p.wg.Done() there and it explicitly mentioned a deadlock. So I reverted this change and replaced it with that comment instead.
bhandras
approved these changes
Nov 23, 2020
docs/DOCKER.md
Outdated
| - `Dockerfile`: Used for production builds. Checks out the source code from | ||
| GitHub during build. The build argument `--build-arg checkout=v0.x.x-beta` | ||
| can be used to specify what git tag or commit to check out before building. | ||
| - `Dockerfile.dev` Used for development or testing builds. Uses the local code |
There was a problem hiding this comment.
Oh. Thanks, fixed!
guggero
force-pushed
the
old-pr-cleanup
branch
from
44288af
to
4f2e8b2
Compare
To finally end the discussion what Dockerfile should be used for what and whether we should build from local source or check out from git, we place both Dockerfiles next to each other and explicitly document their purpose.
guggero
force-pushed
the
old-pr-cleanup
branch
from
4f2e8b2
to
94183e0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains multiple independent changes collected from old, stale PRs.
Replaces:
Original PR authors are attributed where possible.
Because 4 of the 5 replaced PRs somehow involve Docker and development builds, I added a commit of my own to finally clear up this discussion (by tacking my own personal opinion on top of it).