Skip to content
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

Implement LogOutputChannel to improve logging #456

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JustinGrote
Copy link

@JustinGrote JustinGrote commented Mar 18, 2025

Updates the logging API to use LogOutputChannel for automatic severities, user-selectible log levels, and better log management.
image

@JustinGrote JustinGrote marked this pull request as ready for review March 18, 2025 16:52
@Copilot Copilot bot review requested due to automatic review settings March 18, 2025 16:52
@JustinGrote JustinGrote requested a review from a team as a code owner March 18, 2025 16:52
Copy link

@Copilot 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.

Pull Request Overview

This PR implements LogOutputChannel for logging improvements, updates the ESLint configuration, and bumps the Node version for the CI builds while also refactoring several TypeScript error workarounds.

  • Replace old logging functions in src/log.ts with LogOutputChannel methods.
  • Update ESLint config and build workflow (Node version bump).
  • Add/adjust ts-expect-error comments and reformat constructors in various modules to improve type checking with newer TypeScript rules.

Reviewed Changes

Copilot reviewed 21 out of 26 changed files in this pull request and generated no comments.

Show a summary per file
File Description
eslint.config.mjs Introduces a flat ESLint configuration with custom ignore patterns.
src/log.ts Refactors logging to use vscode.LogOutputChannel with specific log methods.
.github/workflows/build.yml Updates node-version from 16.x to 20.x.
src/store/workflowRun.ts Adds ts-expect-error comments for Octokit-related API calls.
src/pinnedWorkflows/pinnedWorkflows.ts Introduces ts-expect-error comments and maps workflows by path.
src/treeViews/icons.ts Refactors functions to return proper types (e.g. undefined instead of "").
src/treeViews/settings/repoSecretsNode.ts Adds ts-expect-error comments around repo secrets API calls.
src/treeViews/settings/environmentSecretsNode.ts Updates constructor formatting and adds ts-expect-error comments.
src/treeViews/settings/environmentVariablesNode.ts Updates constructor formatting and adds ts-expect-error comments.
src/commands/openWorkflowFile.ts Simplifies error handling by removing unused error parameter in catch.
src/treeViews/settings/repoVariablesNode.ts Adds ts-expect-error comments for repo variables API calls.
src/treeViews/shared/previousAttemptsNode.ts Reformats constructor definition for clarity.
src/treeViews/shared/attemptNode.ts Reformats constructor definition for clarity.
src/treeViews/current-branch/currentBranchRepoNode.ts Reformats constructor definition for clarity.
src/treeViews/settings/environmentNode.ts Reformats constructor definition for clarity.
Files not reviewed (5)
  • .eslintrc.json: Language not supported
  • .prettierrc.json: Language not supported
  • .vscode/extensions.json: Language not supported
  • .vscode/settings.json: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (2)

eslint.config.mjs:13

  • [nitpick] Consider renaming 'globalignores' to 'globalIgnores' for better readability and consistency with typical camelCase naming conventions.
name: "globalignores",

src/treeViews/settings/repoVariablesNode.ts:19

  • [nitpick] The comment refers to 'octokit bump' in lowercase. For consistency, consider capitalizing it to 'Octokit bump'.
// @ts-expect-error FIXME: Type error after newer rules available. This will be fixed in octokit bump

@JustinGrote JustinGrote force-pushed the feature/improvedLogging branch from a81a4a9 to 35d3342 Compare March 18, 2025 16:55
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.

1 participant