Skip to content

Conversation

@moliholy
Copy link
Collaborator

@moliholy moliholy commented Oct 29, 2025

Followed by #696, I reliazed composite keys cannot be partially specified. This PR forces the user to specify them instead of raising an error when attempting to query an storage key that is not fully complete, yielding strange errors.

From DevEx perspective, it's hard to justify to the user why sometimes keys can be omitted and why sometimes they are mandatory, but I think it's better to at least let them know that they must specify the full key in this case.

Screenshot 2025-10-29 at 12 06 35

@moliholy moliholy requested a review from AlexD10S October 29, 2025 11:05
@moliholy moliholy self-assigned this Oct 29, 2025
@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.06%. Comparing base (3aab050) to head (3a55635).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/pop-cli/src/commands/call/chain.rs 25.00% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main     #697      +/-   ##
==========================================
- Coverage   77.06%   77.06%   -0.01%     
==========================================
  Files         109      109              
  Lines       24667    24669       +2     
  Branches    24667    24669       +2     
==========================================
+ Hits        19010    19011       +1     
- Misses       3654     3656       +2     
+ Partials     2003     2002       -1     
Files with missing lines Coverage Δ
crates/pop-cli/src/commands/call/chain.rs 67.14% <25.00%> (-0.05%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@moliholy moliholy changed the title feat: force specify parameter if key is composite feat: force specifying storage parameters if the key is composite Oct 29, 2025
Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

LGTM, this makes sense.

@moliholy moliholy merged commit a29ac18 into main Oct 29, 2025
20 checks passed
@moliholy moliholy deleted the feat/force-specify-param branch October 29, 2025 13:40
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.

3 participants