Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the GitIgnoreSDK to change from a Result-based async pattern using neverthrow to a standard Promise-based approach with try/catch error handling. The main change converts the generate method from returning a ResultAsync to using async/await with thrown errors.
- Converted the API from Result-based to Promise-based error handling
- Reorganized domain interfaces by moving HttpClient to a separate file
- Updated documentation and tests to reflect the new Promise-based API
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/gitignoreio-sdk/src/gitignore-sdk.ts | Updated generate method to use async/await and throw errors instead of returning ResultAsync |
| packages/gitignoreio-sdk/src/domain/technology.ts | Renamed GitIgnoreElement to Technology and added Technologies type definition |
| packages/gitignoreio-sdk/src/domain/index.ts | Removed HttpClient interface and updated GitIgnoreIoSDK to use Promise return type |
| packages/gitignoreio-sdk/src/domain/http-client.ts | New file containing the HttpClient interface moved from index.ts |
| packages/gitignoreio-sdk/src/adapters/fetch/client.ts | Updated import path for HttpClient interface |
| packages/gitignoreio-sdk/src/tests/gitignore-sdk.test.ts | Added test for error handling and updated existing test for Promise-based API |
| packages/gitignoreio-sdk/src/tests/data.ts | Updated import path for HttpClient interface |
| packages/gitignoreio-sdk/package.json | Moved neverthrow dependency and added bugs field |
| packages/gitignoreio-sdk/README.md | Updated documentation to show Promise-based usage with try/catch |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const result = await this.httpClient.get(url); | ||
| return result | ||
| .map((content) => ({ content })) | ||
| .match( | ||
| (value) => value, | ||
| (error) => { | ||
| throw error; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
This code can be simplified using ResultAsync.unwrapOr() or similar neverthrow utilities instead of manually handling the match pattern. Consider: return { content: await this.httpClient.get(url).unwrapOrElse((error) => { throw error; }) };
| const result = await this.httpClient.get(url); | |
| return result | |
| .map((content) => ({ content })) | |
| .match( | |
| (value) => value, | |
| (error) => { | |
| throw error; | |
| }, | |
| ); | |
| return { | |
| content: await this.httpClient.get(url).unwrapOrElse((error) => { throw error; }), | |
| }; |
| */ | ||
| export type Technologies = [Technology, ...Technology[]]; | ||
|
|
||
| type Technology = |
There was a problem hiding this comment.
The Technology type should be exported to allow consumers to use it for type checking and validation. Change type Technology = to export type Technology =
| type Technology = | |
| export type Technology = |
c70262c to
3392ca9
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| () => { | ||
| throw new Error('There was an error fetching data from gitignore.io'); |
There was a problem hiding this comment.
The error handling discards the original error information. The match function receives an error parameter that should be used to provide more meaningful error messages or at least preserve the original error context.
| () => { | |
| throw new Error('There was an error fetching data from gitignore.io'); | |
| (error) => { | |
| // Preserve original error information | |
| if (error instanceof Error) { | |
| throw new Error(`There was an error fetching data from gitignore.io: ${error.message}`, { cause: error }); | |
| } else { | |
| throw new Error(`There was an error fetching data from gitignore.io: ${String(error)}`); | |
| } |
This pull request refactors the
gitignoreio-sdkpackage to improve its API usability, error handling, and code organization. The SDK'sgeneratemethod now returns a Promise and throws on errors, rather than using theneverthrowResult type. The domain types have been renamed and clarified, and the codebase has been reorganized to separate adapters and domain logic. Tests and documentation have been updated to reflect these changes.