Skip to content

Scopes#179

Open
philocalyst wants to merge 46 commits into
cococonscious:mainfrom
philocalyst:scopes
Open

Scopes#179
philocalyst wants to merge 46 commits into
cococonscious:mainfrom
philocalyst:scopes

Conversation

@philocalyst

Copy link
Copy Markdown
Contributor

No description provided.

philocalyst and others added 18 commits December 10, 2025 10:13
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/cache](https://redirect.github.com/actions/cache) | action |
major | `v4.3.0` -> `v5.0.1` |

---

### Release Notes

<details>
<summary>actions/cache (actions/cache)</summary>

###
[`v5.0.1`](https://redirect.github.com/actions/cache/compare/v5.0.0...v5.0.1)

[Compare
Source](https://redirect.github.com/actions/cache/compare/v5.0.0...v5.0.1)

###
[`v5.0.0`](https://redirect.github.com/actions/cache/compare/v4.3.0...v5.0.0)

[Compare
Source](https://redirect.github.com/actions/cache/compare/v4.3.0...v5.0.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM, only on
Monday ( * 0-3 * * 1 ) (UTC), Automerge - At any time (no schedule
defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/cococonscious/koji).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi40Mi4yIiwidXBkYXRlZEluVmVyIjoiNDIuNDIuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…us#165)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[codecov/codecov-action](https://redirect.github.com/codecov/codecov-action)
| action | patch | `v5.5.1` -> `v5.5.2` |

---

### Release Notes

<details>
<summary>codecov/codecov-action (codecov/codecov-action)</summary>

###
[`v5.5.2`](https://redirect.github.com/codecov/codecov-action/blob/HEAD/CHANGELOG.md#v552)

[Compare
Source](https://redirect.github.com/codecov/codecov-action/compare/v5.5.1...v5.5.2)

##### What's Changed

**Full Changelog**:
<https://github.com/codecov/codecov-action/compare/v5.5.1..v5.5.2>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM, only on
Monday ( * 0-3 * * 1 ) (UTC), Automerge - At any time (no schedule
defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/cococonscious/koji).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi40Mi4yIiwidXBkYXRlZEluVmVyIjoiNDIuNDIuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/checkout](https://redirect.github.com/actions/checkout) |
action | patch | `v6.0.0` -> `v6.0.1` |

---

### Release Notes

<details>
<summary>actions/checkout (actions/checkout)</summary>

###
[`v6.0.1`](https://redirect.github.com/actions/checkout/compare/v6.0.0...v6.0.1)

[Compare
Source](https://redirect.github.com/actions/checkout/compare/v6.0.0...v6.0.1)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM, only on
Monday ( * 0-3 * * 1 ) (UTC), Automerge - At any time (no schedule
defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/cococonscious/koji).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi4zMi4yIiwidXBkYXRlZEluVmVyIjoiNDIuMzIuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Multiple macOS versions as a requirement to eventually have koji in
homebrew-core
Keeping cocogitto for the commits for now but uses Gix for traversal, etc., and at some point, when Gix is ready, replace cocogitto with it.
@cococonscious

cococonscious commented Feb 25, 2026

Copy link
Copy Markdown
Owner

Besides the conflicts, make sure to consider my comment in the previous PR: #161 (comment) and imo it should also be possible to configure to force the use of the configured scopes or not (and with that if an empty scope is allowed or not)

@codecov

codecov Bot commented Mar 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.73835% with 37 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/lib/questions.rs 89.62% 14 Missing ⚠️
src/lib/scope.rs 88.42% 14 Missing ⚠️
src/lib/config.rs 60.86% 9 Missing ⚠️
Files with missing lines Coverage Δ
src/lib/config.rs 84.74% <60.86%> (-15.26%) ⬇️
src/lib/questions.rs 93.92% <89.62%> (-3.41%) ⬇️
src/lib/scope.rs 88.42% <88.42%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cococonscious cococonscious left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good besides the review comment, also, I have two other points to consider:

  • The force_scope config key naming is a bit misleading, I think a more accurate name would be force_config_scopes
  • We should show the configured scope description in the selection prompt, not only because it's otherwise obsolete but also to differentiate from the detected scopes

Comment thread src/lib/questions.rs Outdated
@philocalyst

philocalyst commented Mar 11, 2026 via email

Copy link
Copy Markdown
Contributor Author

@cococonscious

Copy link
Copy Markdown
Owner

I understand your point about the scoping of the config property, and it does make sense, but it's a bit confusing because "force scope" could be misunderstood as "you must specify a scope" (which the other variable is for so it is understandable, yes, but it does require an extra thinking step).

Thanks for the changes. I did find a bug though, when setting the following: force_scope = false and allow_empty_scope = false, hitting <Esc> will just continue instead of throwing like when force_scope = true.

Also, I find it hard to justify adding another error handling dependency, could we not either replace everything with thiserror or implement your changes in anyhow?

Lastly, the descriptions of the commit scopes defined in the config are still not visible.

Thanks,
Finley

@philocalyst

philocalyst commented Mar 14, 2026 via email

Copy link
Copy Markdown
Contributor Author

@cococonscious

Copy link
Copy Markdown
Owner

I think if we want to move to stronger error handling, the answer is 100% thiserror!! ... I could start a separate PR that adds that to merge before this one so consistency stays.

Yes, please! That would be great.

The other thing we could consider is Ariadne or Miette, ...

While they look great, I think they're overkill for the simple errors that are common in koji, unless you disagree...

I also was thinking that globbing would pair nicely with this scope configuration, ...

Sorry, I'm a bit lost on what exactly you mean by this, could you give me a concrete example?

@philocalyst

philocalyst commented Mar 17, 2026 via email

Copy link
Copy Markdown
Contributor Author

@philocalyst

Copy link
Copy Markdown
Contributor Author

Feeling a lot better about this! @cococonscious

@philocalyst

Copy link
Copy Markdown
Contributor Author

@cococonscious Following up

@cococonscious cococonscious left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I like the general idea, but please do look at my review comments and answer / fix the issues, thanks! Please also don't forget to add unit tests for the new functions, covering both positive and negative cases, for example: looks_like_regex

PS: Sorry for the late replies and reviews 😮‍💨

Comment thread src/lib/questions.rs Outdated
Comment thread Cargo.toml Outdated
config = { version = "0.15", features = ["toml"] }
xdg = "3.0"
gix = "0.80.0"
ast-grep-config = { version = "0.42.1" }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please put these features behind a feature flag. I don't mind if it's in the default features, but I want to be able to exclude it if necessary.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@philocalyst Also not resolved, compile with --no-default-features to check.

Comment thread Cargo.toml
Comment thread src/lib/config.rs Outdated
Comment thread src/lib/config.rs Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated

- Location: In the normal Koji config file alongside the rest of the scope config
- Optional: `true`
- Description: Ast-grep rules can pre-assign commit scopes from structural code matches.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it'd be great if the pre-assigned scope, when an ast-grep rule matches, would show up in the prompt somehow, I think nicest would be something like this:

> What type of change are you committing? feat
> What's the scope of this change? foo      <---- This line should have been printed automatically instead of being omitted.
> Write a short, imperative tense description of the change: bar

If this is not possible, for example if the prompting library doesn't allow for it, print it in some other fitting way, I just don't like that it's invisible.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@philocalyst This is not resolved, it's still not showing the assigned scope:

> What type of change are you committing? feat
> Write a short, imperative tense description of the change: foo
> ...
feat(lib): foo

Comment thread src/bin/main.rs
Comment thread src/lib/scope.rs Outdated
Comment thread src/lib/scope.rs Outdated
}

pub fn detect_scope_matches(repo: &Repository, config: &Config) -> Result<ScopeMatches> {
let changed_paths = staged_paths(repo)?;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is the behavior of this example expected?:

  1. tests/a.rs renamed to src/a.rs
  2. old path (tests/a.rs) matches filter but cannot be read -> skipped
  3. new path (src/a.rs) can be read, but doesn’t match filter -> no rule applied
    => result: no AST scope match, even though the staged change involved a test file rename.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@philocalyst I think this is also still open, if not, please answer with an explanation before resolving.

@philocalyst

Copy link
Copy Markdown
Contributor Author

My own turn for a slow response! Pretty happy with how things are looking right now, wondering if you'd like to try to use it to add a few local rules for Koji itself and see if it's intuitive or not with a real use-case? That could be fun :)

@philocalyst

Copy link
Copy Markdown
Contributor Author

OKAY i think finally solid @cococonscious

i was lowkey intimidated by my own feature so was avoiding testing it directly, my fault for the annoying back and forth

@philocalyst

Copy link
Copy Markdown
Contributor Author

Seems the failing test is due to a missing key @cococonscious

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.

2 participants