-
Notifications
You must be signed in to change notification settings - Fork 89
feat: add HTTP(S) proxy support #310
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
Conversation
|
Looks good - would love this to be merged and released! |
|
Would love to have this feature implemented, exactly what my company requires |
|
I'd like this. Make it happen 🚀 |
|
@travi Reading through the comments in the related MRs and issue I think this is the most promising way forward. I don't see a lot of value in aligning with the GItHub plugin because the code paths we touch here are GitLab-specific anyway and would not be part of the common base. What do you think? |
|
thanks for bringing my attention back to this one, @fgreinacher, and apologies to everyone for the additional delays. this comment brought be around to agreeing that divergence from the github plugin is justified in this case, but i failed to come back to confirm my thoughts or see things through. thank you @naffers for the thorough investigation to raise visibility into the details that made this more clear. @fgreinacher if you could help move this effort forward, i agree that this path makes sense at this point. |
|
@b3nk3 Are you still able to work on this? If so, could you please rebase on latest master so that I can review? |
|
Hey @fgreinacher, I'll take a look tomorrow. |
Done |
fgreinacher
left a comment
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.
Thanks @b3nk3! I left some comments
Co-authored-by: Florian Greinacher <florian@greinacher.de>
Co-authored-by: Florian Greinacher <florian@greinacher.de>
|
Hey @fgreinacher, I think I've covered everything. If there is anything else I may need to pick it up in a week's time. |
|
Thanks a lot for your contribution @b3nk3! |
|
🎉 This PR is included in version 8.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Co-authored-by: Ben Szabo <ben.szabo@hl.co.uk> Co-authored-by: Florian Greinacher <florian.greinacher@siemens.com>
This PR proposes the implementation of a http/https proxy support based on this PR
As pointed out by @alexjfischer on the above PR some extra changes had to be made around gitlab url props, which is included.
This is not my original work, rather a rebase and fixup of the original PR.
I would also like to point out, that we have been running this solution for over 6 months in our production environment.
@travi has raised valid points on the other PR about not taking on tech debt, and recommended the same approach as the GitHub plugin, however based on the extra dependencies and therefore peer dependencies, this implementation is cleaner and poses less risk in support and future tech debt. More info
Thanks in advance for reviewing. Do let me know if I can be of assistance to get this merged in a timely manner.