Update to support private deployment domain name resolution and configuration #350
Conversation
Summary of ChangesHello @anfreshman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the system's ability to identify and handle various Git service providers, particularly those with custom or private domain deployments. By externalizing the repository type detection logic into a configurable JSON file, the system gains greater flexibility and scalability, ensuring accurate recognition of GitHub, GitLab, and Bitbucket instances regardless of their domain structure. This change addresses the challenge of irregular domain names in private Git services, making the system more adaptable. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a more flexible and scalable way to identify repository providers by moving the detection logic into a configurable JSON file, repository-providers.json. This allows for custom domain resolution for private deployments, which is a great improvement. The refactoring in page.tsx and [owner]/[repo]/page.tsx to use the new detectRepositoryType function centralizes the logic and improves maintainability.
My review includes a few suggestions to enhance the implementation:
- A high-severity suggestion to make the new wildcard matching logic more robust by using regular expressions.
- A medium-severity suggestion to improve type safety by defining a proper interface for the new configuration file.
- A minor fix for a typo in the configuration file.
| if (pattern.includes('*')) { | ||
| // Handle wildcards by converting to simple contains/starts/ends checks | ||
| const cleanPattern = pattern.replace(/\*/g, ''); | ||
| if (pattern.startsWith('*') && pattern.endsWith('*')) { | ||
| // *pattern* - contains | ||
| if (hostname.includes(cleanPattern)) return provider.type; | ||
| } else if (pattern.startsWith('*')) { | ||
| // *pattern - ends with | ||
| if (hostname.endsWith(cleanPattern)) return provider.type; | ||
| } else if (pattern.endsWith('*')) { | ||
| // pattern* - starts with | ||
| if (hostname.startsWith(cleanPattern)) return provider.type; | ||
| } | ||
| } else { | ||
| // Exact match | ||
| if (hostname === pattern) return provider.type; | ||
| } |
There was a problem hiding this comment.
The current implementation for wildcard matching is not very robust. It only handles simple cases where a single wildcard is at the beginning, end, or both ends of the pattern. It won't work for patterns like git.*.com or patterns with multiple wildcards.
A more robust approach would be to convert the wildcard pattern into a regular expression. This would make the matching more powerful and align better with the expectation of standard wildcard/glob behavior.
// Convert wildcard pattern to a regex for more robust matching.
const regex = new RegExp(`^${pattern.replace(/\./g, '\\.').replace(/\*/g, '.*')}$`);
if (regex.test(hostname)) {
return provider.type;
}
There was a problem hiding this comment.
I didn't use regular expressions because I think repo domain name resolution is a simple function. It is more important to keep it readable while being extensible than to introduce powerful regular expressions. Of course, if necessary, it is not difficult to use regular expressions here.
| "patterns": [ | ||
| "*gitlab.*" | ||
| ], | ||
| "description": "GitLab. and self-hosted instances" |
There was a problem hiding this comment.
There seems to be a typo in the GitLab provider's description. The period after GitLab should probably be removed for better readability.
| "description": "GitLab. and self-hosted instances" | |
| "description": "GitLab and self-hosted instances" |
| const hostname = extractUrlHostname(url); | ||
| if (!hostname) return 'web'; | ||
|
|
||
| const config = repositoryConfig as { providers: Array<{ type: string; patterns: string[] }>; fallback: string }; |
There was a problem hiding this comment.
Using a type assertion as { ... } works, but it's less maintainable and type-safe than defining a proper interface or type for the repositoryConfig structure.
Consider creating a dedicated type definition for your configuration. This improves readability, enables better autocompletion, and makes it easier to manage the configuration structure as it evolves.
You could define an interface like this, perhaps in a shared types file:
interface RepositoryProvider {
type: string;
name: string;
patterns: string[];
description: string;
}
interface RepositoryProvidersConfig {
providers: RepositoryProvider[];
fallback: string;
comment: string;
}Then you could use it as const config: RepositoryProvidersConfig = repositoryConfig;.
Many domain names of private domain deployed git services are irregular. This pr separates the repo type identification to make it more scalable.