refactor: fix quality gate issues, improve type safety and pagination - #21
Conversation
|
![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 enhances the project's code quality and maintainability by integrating the sonarjs ESLint plugin, refactoring the SyncStatusView diff logic into modular components, and centralizing error handling within SyncManager. It also adds support for paginated file listing in GitLab and introduces a GitHub repository ruleset. Review feedback suggests further optimizing the GitLab file listing by adhering to the service interface, leveraging API-side filtering for the root path, and improving array manipulation performance.
| @@ -56,14 +56,29 @@ export class GitLabService extends BaseGitService implements GitServiceInterface | |||
|
|
|||
| async listFiles(branch: string): Promise<string[]> { | |||
There was a problem hiding this comment.
The implementation of listFiles is missing the optional path parameter defined in the GitServiceInterface. To improve type safety and adhere to the interface, the signature should be updated.
| async listFiles(branch: string): Promise<string[]> { | |
| async listFiles(branch: string, path?: string): Promise<string[]> { |
| .map(item => item.path) | ||
| .filter(p => !this.rootPath || p.startsWith(this.rootPath)); | ||
| while (true) { | ||
| const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/tree?ref=${branch}&recursive=true&per_page=${perPage}&page=${page}`; |
There was a problem hiding this comment.
When this.rootPath is configured, it is more efficient to pass it to the GitLab API using the path parameter. This reduces the amount of data transferred and the number of pages to iterate through, especially in large repositories.
| const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/tree?ref=${branch}&recursive=true&per_page=${perPage}&page=${page}`; | |
| const url = `${this.baseUrl}/api/v4/projects/${encodedProjectId}/repository/tree?ref=${branch}&recursive=true&per_page=${perPage}&page=${page}${this.rootPath ? `&path=${encodeURIComponent(this.rootPath)}` : ''}`; |
| const paths = data | ||
| .filter(item => item.type === 'blob') | ||
| .map(item => item.path) | ||
| .filter(p => !this.rootPath || p.startsWith(this.rootPath)); | ||
|
|
||
| allPaths = allPaths.concat(paths); |
There was a problem hiding this comment.
Using allPaths.push(...paths) is more efficient than allPaths.concat(paths) because it modifies the array in place instead of creating a new array on every iteration. This can be simplified by pushing the filtered results directly.
| const paths = data | |
| .filter(item => item.type === 'blob') | |
| .map(item => item.path) | |
| .filter(p => !this.rootPath || p.startsWith(this.rootPath)); | |
| allPaths = allPaths.concat(paths); | |
| allPaths.push(...data | |
| .filter(item => item.type === 'blob') | |
| .map(item => item.path) | |
| .filter(p => !this.rootPath || p.startsWith(this.rootPath))); |
|
🎉 This PR is included in version 1.0.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
1 participant
Summary of Changes
This PR addresses several SonarCloud quality gate issues, improves type safety, and enhances API reliability.
🛠️ Bug Fixes & Improvements
SyncManagerto eliminateanytypes and use a constructor-injected callback for saving settings.GitLabService.listFilesto support repositories with more than 100 files.GitHubService.listFiles.secretswere incorrectly used in a reusable workflowwithblock.noUncheckedIndexedAccess) in the diff algorithm logic.🧹 Code Quality (SonarJS)
SyncStatusView.tsto reduce cognitive complexity from 29 to under 15 by decomposing large methods.DiffOpandDiffOpTypeto replace complex inline union types.eslint-plugin-sonarjsinto the project's ESLint configuration for continuous quality monitoring.getServiceNamehelper.✅ Verification
npm run lint(Passed)npm run build(Passed)npm run test(Passed)