Fixed wrong key in execute_admin_add_solver function #137
Conversation
removed from cliff.toml the redundant part of first committer
|
Caution Review failedThe pull request is closed. WalkthroughRelease config ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Admin CLI
participant Take as take_dispute.rs
participant Util as util::admin_send_dm
participant Msg as send_dm
CLI->>Take: run admin dispute command
Take->>Util: admin_send_dm(ctx, message)
Util->>Msg: send_dm(ctx.client, ctx.identity_keys, ctx.trade_keys, ctx.pubkey, message)
Msg-->>Util: result
Util-->>Take: propagate result
Take-->>CLI: return outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/cli/take_dispute.rs (1)
23-31: Optional: DRY up repeated send_dm patterns for admin disputes.All admin functions pass the same params except the message. Extract a helper to reduce repetition and future key mix-ups.
// helper (place near other CLI utils) async fn admin_send_dm(ctx: &Context, msg: String) -> anyhow::Result<()> { send_dm(&ctx.client, Some(&ctx.context_keys), &ctx.trade_keys, &ctx.mostro_pubkey, msg, None, false).await?; Ok(()) } // usage inside execute_admin_add_solver / cancel / settle / take admin_send_dm(ctx, take_dispute_message).await?;cliff.toml (1)
80-97: Solid commit_parsers; consider a few regex nits and making Security case-insensitive.These small adjustments improve match accuracy without changing intent.
commit_parsers = [ - { message = "^feat", group = "<!-- 0 -->🚀 Features" }, - { message = "^fix", group = "<!-- 1 -->🐛 Bug Fixes" }, - { message = "^doc", group = "<!-- 3 -->📚 Documentation" }, + { message = "^feat", group = "<!-- 0 -->🚀 Features" }, + { message = "^fix", group = "<!-- 1 -->🐛 Bug Fixes" }, + { message = "^docs?", group = "<!-- 3 -->📚 Documentation" }, { message = "^perf", group = "<!-- 4 -->⚡ Performance" }, { message = "^refactor", group = "<!-- 2 -->🚜 Refactor" }, { message = "^style", group = "<!-- 5 -->🎨 Styling" }, { message = "^test", group = "<!-- 6 -->🧪 Testing" }, { message = "^chore\\(release\\): prepare for", skip = true }, { message = "^chore\\(deps.*\\)", skip = true }, { message = "^chore\\(pr\\)", skip = true }, { message = "^chore\\(pull\\)", skip = true }, - { message = "^chore|^ci", group = "<!-- 7 -->⚙️ Miscellaneous Tasks" }, - { body = ".*security", group = "<!-- 8 -->🛡️ Security" }, + { message = "^(chore|ci|build)", group = "<!-- 7 -->⚙️ Miscellaneous Tasks" }, + { body = "(?i)security", group = "<!-- 8 -->🛡️ Security" }, { message = "^revert", group = "<!-- 9 -->◀️ Revert" }, { message = ".*", group = "<!-- 10 -->💼 Other" }, ]
- Note: With the current changelog template, groups aren’t rendered; only a flat “commits” list is shown. If you want the new grouping to appear, switch the template to iterate commit_groups, e.g.:
# inside [changelog].body template {%- for group in commit_groups -%} ### {{ group.title }} {%- for commit in group.commits -%} * {{ commit.message | split(pat="\n") | first | trim }}{% if commit.remote.username %} by @{{ commit.remote.username }}{% endif %}{% if commit.remote.pr_number %} in [#{{ commit.remote.pr_number }}]({{ self::remote_url() }}/pull/{{ commit.remote.pr_number }}){% endif %} {%- endfor -%} {%- endfor -%}
- Also confirm it’s intentional that commit_preprocessors rewrite “Merge pull request #…” to start with “feat:” so merges show under Features.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cliff.toml(2 hunks)src/cli/take_dispute.rs(1 hunks)
🔇 Additional comments (2)
cliff.toml (1)
45-55: Contributors section tweak looks good.Renaming to “Contributors” and removing first-timer filtering aligns with the PR goals; the surrounding {% if github %} guard keeps this safe when API data is unavailable.
Confirm your CI environment provides sufficient GitHub API access (e.g., GITHUB_TOKEN) so contributors populate; otherwise the section will correctly omit itself.
src/cli/take_dispute.rs (1)
23-31: Correct key: context_keys for AdminAddSolver — verifiedAll send_dm calls in src/cli/take_dispute.rs use ctx.context_keys (lines 23, 51, 79, 112); remaining identity_keys occurrences are in non-admin flows/util.rs and are expected.
Can you explicitate which functions? It does not seem that from cli they only get the message as parameter @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5910 @arkanoider You're absolutely right to question my previous suggestion. Let me be more explicit about what I observed: The admin functions I was referring to are these four functions in
You're correct that from the CLI perspective, they don't just differ by message - they have different input parameters ( What I was observing is that all these functions share an identical send_dm(
&ctx.client,
Some(&ctx.context_keys),
&ctx.trade_keys,
&ctx.mostro_pubkey,
[the_message], // Only this varies
None,
false,
)The commonality is in the
So you're right - my initial suggestion for a helper function was overly aggressive. The current approach is actually quite reasonable, and the key fix (using |
Explain this with an example please! @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 6294 @arkanoider Here's a detailed explanation with examples of the changelog template issue: The ProblemThe current commit_parsers = [
{ message = "^feat", group = "<!-- 0 -->🚀 Features" },
{ message = "^fix", group = "<!-- 1 -->🐛 Bug Fixes" },
{ message = "^doc", group = "<!-- 3 -->📚 Documentation" },
# ... etc
]But the template is ignoring these groups and just iterating over all commits in a flat list: Current template (problematic): The SolutionChange the template to iterate over Fixed template (grouped): Output ComparisonCurrent output (flat list): ## What's Changed
* Add new solver validation feature by @user1 in #123
* Fix authentication bug by @user2 in #124
* Update documentation by @user3 in #125
* Refactor code structure by @user4 in #126With grouped template (organized): ## What's Changed
## 🚀 Features
* Add new solver validation feature by @user1 in #123
## 🐛 Bug Fixes
* Fix authentication bug by @user2 in #124
## 📚 Documentation
* Update documentation by @user3 in #125
## 🚜 Refactor
* Refactor code structure by @user4 in #126The key difference is that |
Can you prepare a commit for this? @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@arkanoider I'll prepare the commit to fix the changelog template. Let me first examine the current template structure and then provide the updated version. Here's the commit that updates the cliff.toml template to use grouped commits instead of a flat list: # git-cliff ~ default configuration file
# https://git-cliff.org/docs/configuration
#
# Lines starting with "#" are comments.
# Configuration options are organized into tables and keys.
# See documentation for more information on available options.
[changelog]
# template for the changelog body
# https://keats.github.io/tera/docs/#introduction
body = """
{%- macro remote_url() -%}
https://github.com/{{ remote.github.owner }}/{{ remote.github.repo }}
{%- endmacro -%}
{% macro print_commit(commit) -%}
{% if commit.remote.pr_title -%}
{%- set commit_message = commit.remote.pr_title -%}
{%- else -%}
{%- set commit_message = commit.message -%}
{%- endif -%}
* {{ commit_message | split(pat="\n") | first | trim }}\
{% if commit.remote.username %} by @{{ commit.remote.username }}{%- endif -%}
{% if commit.remote.pr_number %} in \
[#{{ commit.remote.pr_number }}]({{ self::remote_url() }}/pull/{{ commit.remote.pr_number }}) \
{%- endif %}
{%- endmacro -%}
## What's Changed
{%- if commit_groups -%}
{%- for group, commits in commit_groups %}
{%- if commits | length > 0 %}
### {{ group | trim_start_matches(pat="<!-- ") | trim_end_matches(pat=" -->") }}
{% for commit in commits -%}
{{ self::print_commit(commit=commit) }}
{% endfor -%}
{%- endif -%}
{%- endfor -%}
{%- else -%}
{% for commit in commits -%}
{{ self::print_commit(commit=commit) }}
{%- endfor -%}
{%- endif -%}
{%- if github.contributors | filter(attribute="is_first_time", value=true) | length != 0 %}
{%- raw %}
## New Contributors
{%- endraw -%}
{%- endif %}
{% for contributor in github.contributors | filter(attribute="is_first_time", value=true) %}
* @{{ contributor.username }} made their first contribution in [#{{ contributor.pr_number }}]({{ self::remote_url() }}/pull/{{ contributor.pr_number }})
{%- endfor %}
**Full Changelog**: {{ self::remote_url() }}/compare/{{ previous.version }}...{{ version }}
"""
# The rest of the configuration remains the same...Key changes made:
This will produce organized changelog sections like: ## What's Changed
### 🚀 Features
* Add new solver validation feature by @user1 in #123
### 🐛 Bug Fixes
* Fix authentication bug by @user2 in #124
### 📚 Documentation
* Update documentation by @user3 in #125The template maintains backward compatibility, so if commit_groups isn't populated for any reason, it falls back to the original flat list format. 🏁 Script executed: Length of output: 7834 🚀 Features"
@arkanoider Here's the complete updated # git-cliff ~ configuration file
# https://git-cliff.org/docs/configuration
[remote.github]
owner = "MostroP2P"
repo = "mostro-cli"
[changelog]
# A Tera template to be rendered for each release in the changelog.
# See https://keats.github.io/tera/docs/#introduction
body = """
## Verifying the Release
In order to verify the release, you'll need to have gpg or gpg2 installed on your system. Once you've obtained a copy (and hopefully verified that as well), you'll first need to import the keys that have signed this release if you haven't done so already:
```bash
curl https://raw.githubusercontent.com/MostroP2P/mostro/main/keys/negrunch.asc | gpg --importOnce you have the required PGP keys, you can verify the release (assuming manifest.txt.sig and manifest.txt are in the current directory) with: gpg --verify manifest.txt.sig manifest.txt
gpg: Firmado el jue 03 ago 2023 15:07:05 -03
gpg: usando RSA clave 1E41631D137BA2ADE55344F73852B843679AD6F0
gpg: Firma correcta de "Francisco Calderón <fjcalderon@gmail.com>" [absoluta]
That will verify the signature of the manifest file, which ensures integrity and authenticity of the archive you've downloaded locally containing the binaries. Next, depending on your operating system, you should then re-compute the sha256 hash of the archive with What's Changed{%- if version %} in {{ version }}{%- endif -%} {{ group | trim_start_matches(pat="") }}{% for commit in commits -%}
{%- if github -%} Contributors{%- endif %}
{% if version %} {%- macro remote_url() -%} Remove leading and trailing whitespaces from the changelog's body.trim = true A Tera template to be rendered as the changelog's footer.See https://keats.github.io/tera/docs/#introductionfooter = """ """ An array of regex based postprocessors to modify the changelog.Replace the placeholder
|
@grunch ,
cliff.tomland added the emoticons for different parts as in mostro and mostro-coreNow commit with changelog is created by github actions and then committed remotely, so in the end there is one more automatic commit, but we removed git cliff boilerplate from
cargo.toml. Let me know if you like this or we can use the same trick everywhere and let the commit be made locally without the automatic one.This new way has pros:
cons:
in both cases then we can have releases autobuild with nice changelog!
Summary by CodeRabbit
Documentation
Chores
Refactor