Skip to content

app: clone module account permissions - #2221

Open
zjubiology wants to merge 1 commit into
crypto-org-chain:mainfrom
zjubiology:my_feature
Open

zjubiology wants to merge 1 commit into
crypto-org-chain:mainfrom
zjubiology:my_feature

Conversation

@zjubiology

@zjubiology zjubiology commented Sep 26, 2026 •

Copy link
Copy Markdown

Return a clone of the module-account permission map with maps.Clone instead of rebuilding it manually.

This preserves the defensive-copy behavior, makes ownership explicit, and avoids exposing shared configuration to caller mutation.

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (make)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (make test)
  • Have you checked your code formatting is correct? (go fmt)
  • Have you checked your basic code style is fine? (golangci-lint run)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
  • If your changes affect the client infrastructure, have you run the integration test?
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

Summary by CodeRabbit

  • Refactor
    • Internal permission handling has been streamlined. Permission information continues to be returned as an independent copy, with no change to the app’s behavior or available functionality.

@zjubiology
zjubiology requested a review from a team as a code owner September 26, 2026 23:39
@github-actions

Copy link
Copy Markdown
Contributor

@zjubiology your pull request is missing a changelog!

@github-actions

Copy link
Copy Markdown
Contributor

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

Unknown release type "app" found in pull request title "app: clone module account permissions".

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5598871c-4508-4530-b53f-4e7a62968a34

📥 Commits

Reviewing files that changed from the base of the PR and between 2fd0045 and b1c86b9.

📒 Files selected for processing (1)
  • app/app.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

GetMaccPerms now uses maps.Clone to return a shallow copy of the module-account permissions map.

Changes

Macc permissions copy

Layer / File(s) Summary
Clone macc permissions map
app/app.go
GetMaccPerms uses maps.Clone instead of a manual map-copy loop. The file imports the maps package.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to b1c86

This refactor does not change permission-map behavior and is suitable to merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: cloning module account permissions in app.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: zjubiology <zjubiology@outlook.com>
@zjubiology

Copy link
Copy Markdown
Author

@JayT106 I’ve updated the PR to match the repository conventions:

  • changed the title to use the refactor conventional-commit type;
  • added the required changelog entry;
  • kept the change scoped to the existing GetMaccPerms refactor with no behavioral change.

The branch should now be ready for review when you have a chance. Thanks!

This branch has not been deployed

No deployments
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