Skip to content

Version 0.2.0 to support codeowners-rs implementation#22

Open
pbomb wants to merge 1 commit intorubyatscale:mainfrom
pbomb:pbomb-codeowners
Open

Version 0.2.0 to support codeowners-rs implementation#22
pbomb wants to merge 1 commit intorubyatscale:mainfrom
pbomb:pbomb-codeowners

Conversation

@pbomb
Copy link

@pbomb pbomb commented Oct 14, 2025

Allows configuration of the command and file argument so that it can work with the Rust-based implementation of codeowners which has a different file argument (for-file vs for_file) and possibly located at a different location

This includes a minor bump of the version.

@github-project-automation github-project-automation bot moved this to Triage in Modularity Oct 14, 2025
@dduugg dduugg requested a review from a team March 5, 2026 17:53
Copy link
Contributor

@dduugg dduugg left a comment

Choose a reason for hiding this comment

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

Review

Goal

Adds two VS Code configuration settings (command and fileArg) so the extension can work with either the Ruby code_ownership CLI (bin/codeownership for_file) or the Rust codeowners-rs CLI (which uses a different binary location and for-file argument syntax).


Implementation

  • package.json: Adds a configuration block declaring two user-configurable settings:
    • code-ownership-vscode.command (default: bin/codeownership)
    • code-ownership-vscode.fileArg (default: for_file)
  • src/extension.ts: Reads these settings in two places — checkConfiguration() (binary existence check) and runOwnershipCheck() (command invocation).
  • CHANGELOG.md / README.md: Updated to document the change and mention Rust support.

Issues

Bug: configuration block is misplaced in package.json

The configuration block is placed at the top level of package.json rather than inside contributes. VS Code only picks up settings declared under contributes.configuration. As written, the settings won't appear in VS Code's settings UI and config.get() will always return the hardcoded defaults — making the feature silently non-functional.

It should be:

"contributes": {
  "configuration": {
    "type": "object",
    "properties": { ... }
  },
  "commands": [ ... ]
}

Minor: checkConfiguration doesn't handle non-path commands

checkConfiguration does resolve(workspace, command) and checks existsSync. This works fine for relative paths like bin/codeownership, but if a user sets command to an absolute path or a $PATH binary (e.g. codeowners), existsSync will return false and the extension will report itself as unconfigured. Worth noting as a known limitation or handling separately.

Minor: config.get is called redundantly

vscode.workspace.getConfiguration('code-ownership-vscode') and both .get() calls are duplicated between checkConfiguration and runOwnershipCheck. Extracting them to a shared helper would be cleaner.


Merge Conflict

This PR currently has a merge conflict with main and will need to be rebased before it can be merged.


## [Unreleased]

## [0.2.0] - 2025-10-14
Copy link
Contributor

@dduugg dduugg Mar 5, 2026

Choose a reason for hiding this comment

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

Suggested change
## [0.2.0] - 2025-10-14
## [0.2.0] - 2026-03-06


## [0.2.0] - 2025-10-14

- Allow the command and file argument to be configured, so that it can work with the [Codeowners](https://github.com/rubyatscale/codeowners-rs) Rust-based implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Allow the command and file argument to be configured, so that it can work with the [Codeowners](https://github.com/rubyatscale/codeowners-rs) Rust-based implementation.
- Allow the command and file argument to be configured, so that it can work with the [Codeowners](https://github.com/rubyatscale/codeowners-rs) Rust-based implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants