Skip to content

Version 0.2.0 to support codeowners-rs implementation#22

Closed
pbomb wants to merge 1 commit into
rubyatscale:mainfrom
pbomb:pbomb-codeowners
Closed

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

Conversation

@pbomb

@pbomb pbomb commented Oct 14, 2025

Copy link
Copy Markdown

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

@dduugg dduugg left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread CHANGELOG.md

## [Unreleased]

## [0.2.0] - 2025-10-14

@dduugg dduugg Mar 5, 2026

Copy link
Copy Markdown
Contributor

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

Comment thread CHANGELOG.md

## [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
Copy Markdown
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.

@dduugg

dduugg commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, @pbomb — making the extension configurable to work with either the Ruby code_ownership CLI or codeowners-rs is a great idea, especially with the org moving toward the Rust implementation.

It's been about four months since the last review with no follow-up, so I'm closing this for now to keep the queue tidy. The one thing worth flagging clearly: the review found that the new configuration block is placed at the top level of package.json rather than nested under contributes, which means VS Code won't actually pick up the settings — right now the feature would silently no-op and always fall back to the hardcoded defaults. That, plus rebasing to resolve the merge conflict with main, are the main things to sort out before this is ready.

Please feel free to reopen once you've had a chance to address that — thanks again for the contribution!

@dduugg dduugg closed this Jul 25, 2026
@github-project-automation github-project-automation Bot moved this from Triage to Done in Modularity Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants