[projects] allow only github.com and gitlab.com #5113
Conversation
| @@ -341,7 +341,8 @@ function GitProviders(props: { | |||
|
|
|||
| useEffect(() => { | |||
| (async () => { | |||
| setAuthProviders(await getGitpodService().server.getAuthProviders()); | |||
| const providers = await getGitpodService().server.getAuthProviders(); | |||
| setAuthProviders(providers.filter(p => ["github.com", "gitlab.com"].includes(p.host))); | |||
There was a problem hiding this comment.
question: Does this limit Projects for repositories only from github.com and gitlab.com? I thought Projects could support repositories from self hosted GitHub and GitLab instances, but this is also something we could leave out of the scope of this first iteration. 💭
There was a problem hiding this comment.
- The current GitHub App integration is hard coded with github.com.
- I'm new to the GitLab integration for prebuilds in general. It would require testing as well.
There was a problem hiding this comment.
Aha! Thanks for the insight, @AlexTugarev! Sounds good to leave this out of the scope for now until further demand. Added #5115 to track this feature request.
There was a problem hiding this comment.
This would still need an approval from a dashboard owner. Looping in @jankeromnes and @svenefftinge in case any of them can take a look at this two-line change. 🏓
There was a problem hiding this comment.
Fixing this in #5192 😁 also taking a quick look!
There was a problem hiding this comment.
Many thanks! The code seems to do what's intended, but I don't understand why this change is needed.
Could you please elaborate?
Also, @gtsiolis makes a great point about self-managed GitLabs. I'm aware that we haven't tested them and they might break, but maybe it's still preferable to have something that might "accidentally work" for self-managed GitLabs, rather than something that "definitely fails" (i.e. it's filtered out)?
Still, approving in case you feel this change is really necessary. 😊 Maybe I'm missing something.
/hold
/lgtm
/approve no-issue
|
LGTM label has been added. Git tree hash: 6549ee27944cd231da149ac74d1af4c06c63c5e0
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jankeromnes Associated issue requirement bypassed by: jankeromnes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@jankeromnes, I'm not sure about the chances that it work nicely out-of-the box with self-manages GitLabs. It sure does not with GitHub Enterprise. But in general, we should deliver tested feature only. |
|
/unhold |
No description provided.