Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes MinInflation type not found on polkadart_cli #491

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

leonardocustodio
Copy link
Owner

@leonardocustodio leonardocustodio commented Oct 29, 2024

PR Type

enhancement, configuration changes


Description

  • Enhanced the V14Parser by adding new codecs to the _resultingRegistry, including MinInflation, MaxInflation, Falloff, IdealStake, and UseAuctionSlots.
  • Updated the pubspec.yaml file to increment the package version to 1.2.3 and changed the homepage and repository URLs to a new GitHub location.

PRDescriptionHeader.CHANGES_WALKTHROUGH

Relevant files
Enhancement
v14_parser.dart
Add new codecs to the V14Parser registry                                 

packages/substrate_metadata/lib/parsers/v14_parser.dart

  • Added new codecs to the _resultingRegistry.
  • Included codecs for MinInflation, MaxInflation, Falloff, IdealStake,
    and UseAuctionSlots.
  • +6/-0     
    Configuration changes
    pubspec.yaml
    Update package version and repository information               

    packages/substrate_metadata/pubspec.yaml

  • Updated package version from 1.2.2 to 1.2.3.
  • Changed homepage and repository URLs.
  • +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Clarity
    The comment on line 35 suggests a TODO that might be overlooked. Ensure that the reference to the external PR is either resolved or updated to reflect the current state of the code.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Ensure specialized codec instances are used for new codec types to handle specific encoding/decoding behaviors

    Ensure that the ProxyCodec instance is suitable for the new codec types added
    ('MinInflation', 'MaxInflation', 'Falloff', 'IdealStake', 'UseAuctionSlots'). If
    these types require specific encoding/decoding behaviors, consider implementing or
    using specialized codec instances for each.

    packages/substrate_metadata/lib/parsers/v14_parser.dart [32-40]

     {
       final proxyCodec = ProxyCodec();
    +  final minInflationCodec = MinInflationCodec();
    +  final maxInflationCodec = MaxInflationCodec();
    +  final falloffCodec = FalloffCodec();
    +  final idealStakeCodec = IdealStakeCodec();
    +  final useAuctionSlotsCodec = UseAuctionSlotsCodec();
       _resultingRegistry.addCodec('Call', proxyCodec);
       _resultingRegistry.addCodec('RuntimeCall', proxyCodec);
    -  _resultingRegistry.addCodec('MinInflation', proxyCodec);
    -  _resultingRegistry.addCodec('MaxInflation', proxyCodec);
    -  _resultingRegistry.addCodec('Falloff', proxyCodec);
    -  _resultingRegistry.addCodec('IdealStake', proxyCodec);
    -  _resultingRegistry.addCodec('UseAuctionSlots', proxyCodec);
    +  _resultingRegistry.addCodec('MinInflation', minInflationCodec);
    +  _resultingRegistry.addCodec('MaxInflation', maxInflationCodec);
    +  _resultingRegistry.addCodec('Falloff', falloffCodec);
    +  _resultingRegistry.addCodec('IdealStake', idealStakeCodec);
    +  _resultingRegistry.addCodec('UseAuctionSlots', useAuctionSlotsCodec);
     }
    Suggestion importance[1-10]: 7

    Why: The suggestion addresses a potential issue where the same ProxyCodec instance is used for multiple codec types, which may not be suitable if these types require specific encoding/decoding behaviors. Implementing specialized codec instances for each type could enhance the functionality and correctness of the code.

    7

    @leonardocustodio
    Copy link
    Owner Author

    This PR fixes #490 while #487 is not finished and merged.

    @leonardocustodio leonardocustodio changed the title Update v14_parser.dart Fixes MinInflation type not found on polkadart_cli Oct 29, 2024
    Copy link

    codecov bot commented Oct 29, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 48.48%. Comparing base (ce390f2) to head (da86640).
    Report is 1 commits behind head on main.

    Additional details and impacted files

    Impacted file tree graph

    @@             Coverage Diff             @@
    ##             main     #491       +/-   ##
    ===========================================
    - Coverage   96.87%   48.48%   -48.39%     
    ===========================================
      Files           5      181      +176     
      Lines         160     9588     +9428     
    ===========================================
    + Hits          155     4649     +4494     
    - Misses          5     4939     +4934     
    Flag Coverage Δ
    polkadart 15.89% <ø> (?)
    polkadart_cli 12.04% <ø> (?)
    polkadart_keyring 65.24% <ø> (?)
    polkadart_scale_codec 54.93% <ø> (?)
    secp256k1_ecdsa 89.60% <ø> (?)
    sr25519 85.91% <ø> (?)
    ss58 96.87% <ø> (ø)
    substrate_bip39 56.37% <ø> (?)
    substrate_metadata 87.72% <100.00%> (?)

    Flags with carried forward coverage won't be shown. Click here to find out more.

    Files with missing lines Coverage Δ
    ...ges/substrate_metadata/lib/parsers/v14_parser.dart 98.68% <100.00%> (ø)

    ... and 175 files with indirect coverage changes

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants