-
Notifications
You must be signed in to change notification settings - Fork 836
fix(git): add more robust error handling for Git operation #579
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not add a new utils file. Parent
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue #573 mentioned creating a new file, so should I revert the multi-platform refactoring change?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not saying not to do it, just to do it a bit differently than what the issue suggests. However lets keep the PRs one issue at a time. This looks good now. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import { execSync } from "child_process"; | ||
|
|
||
| export function configureGitCredentials(token: string | undefined, repoUrl: string) { | ||
| if (!token) { | ||
| console.warn("No token provided. Skipping Git credential configuration."); | ||
| return false; | ||
| } | ||
| try { | ||
| execSync(`git remote set-url origin ${repoUrl}`, { stdio: "inherit" }); | ||
|
|
||
| return true; | ||
| } catch (error: any) { | ||
| console.error(`Failed to configure Git credentials: ${error.message}`); | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,8 @@ | ||||||
| import { Octokit } from "octokit"; | ||||||
| import { PlatformKit } from "./_base.js"; | ||||||
| import Z from "zod"; | ||||||
| import { execSync } from "child_process"; | ||||||
|
|
||||||
| import { configureGitCredentials } from "./git-utils.js"; | ||||||
|
|
||||||
| export class GitHubPlatformKit extends PlatformKit { | ||||||
| private _octokit?: Octokit; | ||||||
|
|
@@ -76,9 +77,9 @@ export class GitHubPlatformKit extends PlatformKit { | |||||
| if (ghToken && processOwnCommits) { | ||||||
| console.log("Using provided GH_TOKEN. This will trigger your CI/CD pipeline to run again."); | ||||||
|
|
||||||
| execSync(`git remote set-url origin https://${ghToken}@github.com/${repositoryOwner}/${repositoryName}.git`, { | ||||||
| stdio: "inherit", | ||||||
| }); | ||||||
| const URL = `https://${ghToken}@github.com/${repositoryOwner}/${repositoryName}.git`; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use lowercase "url" for variables like this:
Suggested change
|
||||||
|
|
||||||
| configureGitCredentials(ghToken, URL); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for running the prettier!