Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fixes error message when _ is used without dev mode #13886

Merged
merged 10 commits into from
Apr 12, 2023

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Apr 12, 2023

As discussed in #13815

@gupnik gupnik added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 12, 2023
@gupnik gupnik requested review from sam0x17, bkchr and ggwpez April 12, 2023 04:56
@gupnik
Copy link
Contributor Author

gupnik commented Apr 12, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Apr 12, 2023

@gupnik https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2669807 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-f7fb614c-97d6-42d5-9ad6-e6d78cc10fe9 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 12, 2023

@gupnik Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2669807 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2669807/artifacts/download.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Ty

Ok(true)
} else {
let msg = "`_` can only be used in dev_mode. Please specify an appropriate hasher.";
Err(syn::Error::new(args_span, msg))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Err(syn::Error::new(args_span, msg))
Err(syn::Error::new(arg.span(), msg))

To highlight exactly which argument we mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thank you.

@gupnik gupnik merged commit 2b9ca86 into master Apr 12, 2023
@gupnik gupnik deleted the gupnik/optional_hashers_dev branch April 12, 2023 08:26
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Initial changes

* Adds UI test for error when _ is used without dev_mode

* Minor

* ".git/.scripts/commands/fmt/fmt.sh"

* Adds test to verify hasher

* Fixes error message when _ is used without dev mode

* Updates test

* Addresses review comment

---------

Co-authored-by: command-bot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants