Skip to content

refactor(action): reduce redundancy in gitConfig for GitHub, GitLab and BitBucket#581

Closed
giteshsarvaiya wants to merge 1 commit intolingodotdev:mainfrom
giteshsarvaiya:main
Closed

refactor(action): reduce redundancy in gitConfig for GitHub, GitLab and BitBucket#581
giteshsarvaiya wants to merge 1 commit intolingodotdev:mainfrom
giteshsarvaiya:main

Conversation

@giteshsarvaiya
Copy link
Copy Markdown

Solves Issue: #573

Description:

[Refactor] This PR refactors the gitConfig implementation in the ./action/src directory for GitHub, GitLab, and Bitbucket, reducing code redundancy.
It also introduces error handling using try-catch blocks for improved reliability.

Copy link
Copy Markdown
Contributor

@mathio mathio left a comment

Choose a reason for hiding this comment

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

This looks like a duplicate of #579. Please see my comments there.

@mathio
Copy link
Copy Markdown
Contributor

mathio commented Mar 27, 2025

The other PR was merged, however it was updated not to contain the gitConfig refactoring.

Let's not add a new utils file. Parent PlatformKit class has gitConfig() method too. It can be used for any common logic and called using super.gitConfig(...) from its child classes.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

action/src/platforms/bitbucket.ts:96

  • Defaulting BITBUCKET_GIT_HTTP_ORIGIN to 'origin' without validating the environment variable may mask potential misconfigurations. Consider adding validation or logging a warning when the variable is not explicitly set to ensure the intended behavior.
const origin = process.env.BITBUCKET_GIT_HTTP_ORIGIN || "origin";

@@ -91,11 +91,21 @@ export class GitlabPlatformKit extends PlatformKit {
}

gitConfig(): Promise<void> | void {
Copy link

Copilot AI Apr 4, 2025

Choose a reason for hiding this comment

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

The gitConfig function currently has a mixed return type (Promise | void) and relies on console error logging to signal failure. Consider standardizing the return type and error handling approach (for example, by throwing an exception or consistently returning a boolean) across all platform implementations.

Suggested change
gitConfig(): Promise<void> | void {
async gitConfig(): Promise<void> {

Copilot uses AI. Check for mistakes.
@mathio
Copy link
Copy Markdown
Contributor

mathio commented Apr 22, 2025

Closing as outdated. Feel free to reopen once the comments are addressed and conflicts resolved.

@mathio mathio closed this Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants