Skip to content
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

feat: allow for providing an alternative domain for the proxy #749

Merged
merged 6 commits into from
Oct 31, 2024

Conversation

06kellyjac
Copy link
Contributor

@06kellyjac 06kellyjac commented Oct 17, 2024

  • feat: allow for providing an alternative domain for the proxy
    • Have the service report back the proxyURL for each project, rather than
    • being embedded inside the frontend bundle.
    • Concequence is duplicating some data but very minor and allows for us to serve
    • different projects from different proxies in the future.
    • Removed unused vite env var, added method to pull service's current path and substitute
    • in the http proxy port. Works for local dev, should maintain current behaviour.
    • Custom domain can be applied via optional config.
    • New config value added to the config schema.
    • Config schema documentation has been regenerated (including some other absent items).

Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 0174814
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/67237b58c5f9710007c84b7a

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.23%. Comparing base (00a9d12) to head (0174814).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/service/routes/repo.js 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #749      +/-   ##
==========================================
+ Coverage   60.08%   60.23%   +0.14%     
==========================================
  Files          46       47       +1     
  Lines        1631     1647      +16     
==========================================
+ Hits          980      992      +12     
- Misses        651      655       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@JamieSlome JamieSlome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR, cleanly implemented, just a bit of cleanup and test coverage and we're good to go ❤️

flake.lock Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
src/service/proxyURL.js Outdated Show resolved Hide resolved
src/service/routes/push.js Outdated Show resolved Hide resolved
src/service/routes/push.js Outdated Show resolved Hide resolved
src/service/routes/repo.js Outdated Show resolved Hide resolved
src/ui/views/RepoDetails/RepoDetails.jsx Outdated Show resolved Hide resolved
src/ui/views/RepoList/Components/RepoOverview.jsx Outdated Show resolved Hide resolved
website/docs/configuration/reference.mdx Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
@JamieSlome
Copy link
Member

@06kellyjac - what would you recommend in terms of version bump, patch or minor?

@06kellyjac
Copy link
Contributor Author

With a new capability I'd say minor 👍

Have the service report back the proxyURL for each project, rather than
being embedded inside the frontend bundle.
Concequence is duplicating some data but very minor and allows for us to serve
different projects from different proxies in the future.
Removed unused vite env var, added method to pull service's current path and substitute
in the http proxy port. Works for local dev, should maintain current behaviour.
Custom domain can be applied via optional config.
New config value added to the config schema.
Config schema documentation has been regenerated (including some other absent items).
Copy link
Member

@JamieSlome JamieSlome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🍰

Rockstar contribution 🎸

@JamieSlome JamieSlome merged commit a91570e into finos:main Oct 31, 2024
14 checks passed
@06kellyjac 06kellyjac deleted the allow_custom_proxy_domain branch October 31, 2024 16:27
Psingle20 pushed a commit to Psingle20/git-proxy that referenced this pull request Nov 27, 2024
feat: allow for providing an alternative domain for the proxy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants