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.

Sign up for GitHub

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

[projects] allow only github.com and gitlab.com #5113

Merged
merged 1 commit into from
Aug 19, 2021
Merged

[projects] allow only github.com and gitlab.com #5113

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion components/dashboard/src/projects/NewProject.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Copy link
Contributor

@gtsiolis gtsiolis Aug 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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. 💭

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. The current GitHub App integration is hard coded with github.com.
  2. I'm new to the GitLab integration for prebuilds in general. It would require testing as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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. 🏓

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixing this in #5192 😁 also taking a quick look!

})();
}, []);

Expand Down