DO NOT MERGE#599
Conversation
ghost
mentioned this pull request
![echolayer-dev-karl[bot]](https://avatars.githubusercontent.com/u/85580729?s=60&v=4){kind=small}
| } catch (error) { | ||
| // Couldn't parse object ¯\_(ツ)_/¯ | ||
| } |
There was a problem hiding this comment.
The catch block on lines 7-9 should log the error to avoid silent failures during JSON stringification of the meta object. Consider using console.error or the winston logger itself to record the error.
| normalizeDepth: 10, | ||
| }, | ||
| level: "error", | ||
| level: process.env.SENTRY_LOG_LEVEL || "error", |
There was a problem hiding this comment.
Ensure that the environment variable SENTRY_LOG_LEVEL is properly documented and validated to prevent invalid log levels from being used, which could cause runtime issues.
| return originRepo; | ||
| } | ||
|
|
||
| logger.info(`Could not find repo origin - gitService.getFetchOrigin`, { |
There was a problem hiding this comment.
The log message in line 17 references a method getFetchOrigin that has been renamed to getRemote. Update the log message to reflect the correct method name to avoid confusion during debugging.
| repositoryRemotes: repository?.state?.remotes, | ||
| }); | ||
| // We couldn't find a repo w/ Origin as it's name, so we'll naively take the first one | ||
| originRepo = repository?.state?.remotes && repository?.state?.remotes[0]; |
There was a problem hiding this comment.
Consider adding a null check for repository?.state?.remotes before accessing the first element in line 21 to prevent potential runtime errors if remotes is undefined.
|
|
||
| static toVSCodeRepository(repository: Repository): VSCodeRepository | undefined { | ||
| const fetchOrigin = GitService.getFetchOrigin(repository); | ||
| if (!fetchOrigin) return; | ||
|
|
||
| const parsedRemoteGitUrl = gitUrlParse(fetchOrigin.fetchUrl); | ||
| static toVSCodeRepository(repository: Repository, remote: Remote): VSCodeRepository | undefined { | ||
| const parsedRemoteGitUrl = gitUrlParse(remote.fetchUrl); |
There was a problem hiding this comment.
The refactoring of toVSCodeRepository to take a Remote object directly simplifies the method and removes the need for a null check on fetchOrigin. Ensure that all calls to this method are updated to pass the Remote object as the second argument.
| const repository = this.gitAPI.getRepository(uri); | ||
| const fetchOrigin = GitService.getFetchOrigin(repository); | ||
| if (fetchOrigin) { | ||
| const vscodeRepository: VSCodeRepository = GitService.toVSCodeRepository(repository); | ||
| const remote = GitService.getRemote(repository); | ||
| if (remote) { | ||
| const vscodeRepository: VSCodeRepository = GitService.toVSCodeRepository(repository, remote); |
There was a problem hiding this comment.
The refactoring from getFetchOrigin to getRemote and the corresponding parameter change in toVSCodeRepository method improves clarity and maintainability by using a more descriptive method name and passing the relevant Remote object directly.
| const remote = GitService.getRemote(repository); | ||
| if (remote?.fetchUrl) { | ||
| const vscodeRepository: VSCodeRepository = GitService.toVSCodeRepository(repository, remote); |
There was a problem hiding this comment.
The refactoring to use GitService.getRemote(repository) improves code readability by abstracting the logic for obtaining the remote. However, ensure that the toVSCodeRepository method is updated to accept the new remote parameter as it now requires two arguments.
|
@echolayer-karl /ship |
2 participants
Pull Request Summary by EchoLayer
This pull request introduces a new SENTRY_LOG_LEVEL environment variable to fine-tune Sentry logging, and updates the error logging mechanism in
logging.helper.tsto include JSON stringification of metadata, leveraging the new environment variable for setting the log level. Additionally,git.service.tshas been refactored to introduce agetRemotemethod for more reliable fetching of repository remotes, and thetoVSCodeRepositoryfunction has been modified to accept a remote parameter, ensuring that the origin remote is always checked before falling back to the first available remote.File Changes
.env.example: Added SENTRY_LOG_LEVEL environment variable with a default value of 'error'.logging.helper.ts: Enhanced error logging with JSON stringification of metadata and set default Sentry log level from environment variable.git.service.ts: Refactored to use a new getRemote method for fetching repository remotes and updated toVSCodeRepository to accept a remote parameter.