Conversation
|
Still thinking about an integration test for this, to ensure that it always works. I'm happy to do that as a second PR. Keen to hear from reviewers though. |
johnstcn
left a comment
johnstcn
left a comment
There was a problem hiding this comment.
Nice work! Just a couple of nits. I especially like the demonstration you provided, it really aids understanding.
Separate PR is fine 👍 but always a fan of an integration test for this sort of functionality. |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
Nice work on this documentation! There are a few things that I found that I think can be improved, but there's two thoughts I'd like to leave you with that I think can be beneficial to your writing in the future:
- Can this be said in less words, i.e. is this critical to get the point across?
- Can this be made more concrete?
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
mafredri
changed the title
mafredri
changed the title
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
|
@mafredri I've applied all of your review notes. I'm going ahead and merging this. If you would like to provide further feedback, I will open another PR to update :) Thanks for the review. |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
Great work on the rewrite, looks good. Spotted a stray deletion without compensation tho 😄
| As before, this command yields a shell inside an Envbuilder built environment. Feel free to test it and then exit the container. Assuming this worked, Envbuilder will have cloned a repository and built the relevant container using a proxy that required accepting a custom CA certificate. | ||
|
|
||
| ### Bonus | ||
| To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image. |
There was a problem hiding this comment.
| To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image. | |
| To prove that Envbuilder did in fact use the proxy, and also because it is interesting to observe, open `http://localhost:8081/` in your local browser and you see the mitmproxy web interface. In the flow tab, there will be a list of all of the HTTP requests that were required to build the container. The first few requests will be those used to clone the Git repository. The rest will be the requests that were used to pull the devcontainer image. |
3 participants
This PR closes #411