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

chore: improve error message for blocked params #1709

Merged
merged 2 commits into from
May 8, 2023

Conversation

NishantKoyalwar
Copy link
Contributor

@NishantKoyalwar NishantKoyalwar commented May 5, 2023

Overview

  • updated err message to
var ErrBlockedParameter = sdkerrors.Register(ModuleName, baseErrorCode, "parameter can not be modified")

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@MSevey MSevey requested a review from a team May 5, 2023 21:07
@MSevey MSevey requested review from staheri14 and removed request for a team May 5, 2023 21:07
rootulp
rootulp previously approved these changes May 5, 2023
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

thanks @NishantKoyalwar !

var ErrBlockedParameter = sdkerrors.Register(ModuleName, baseErrorCode, "blocked parameter change, hard fork required")
var ErrBlockedParameter = sdkerrors.Register(ModuleName, baseErrorCode, "parameter can not be modified")
Copy link
Member

Choose a reason for hiding this comment

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

the CI will break cause we hardcoded a test, so we should also update

require.Contains(t, err.Error(), "blocked parameter change")

@codecov-commenter
Copy link

Codecov Report

Merging #1709 (86ffcb5) into main (2dc35d9) will decrease coverage by 0.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1709      +/-   ##
==========================================
- Coverage   52.71%   52.58%   -0.14%     
==========================================
  Files          97       97              
  Lines        6269     6278       +9     
==========================================
- Hits         3305     3301       -4     
- Misses       2631     2644      +13     
  Partials      333      333              

see 1 file with indirect coverage changes

@rootulp rootulp changed the title update err msg of blocked params chore: improve error message for blocked params May 8, 2023
@rootulp rootulp added the chore optional label for items that follow the `chore` conventional commit label May 8, 2023
@rootulp rootulp merged commit e223493 into celestiaorg:main May 8, 2023
@gitpoap-bot
Copy link

gitpoap-bot bot commented May 8, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Celestia Contributor:

GitPOAP: 2023 Celestia Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@NishantKoyalwar NishantKoyalwar deleted the rename-error branch May 10, 2023 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore optional label for items that follow the `chore` conventional commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update error message for Blocked params
5 participants