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

Restructure frame_benchmarking macro related exports #14787

Merged

Conversation

juangirini
Copy link
Contributor

@juangirini juangirini commented Aug 18, 2023

@juangirini
Copy link
Contributor Author

bot rebase

…orts' into jg/restructure-benchmarking-macro-related-exports
@paritytech-processbot
Copy link

Rebased

Base automatically changed from jg/restructure-macro-related-exports to master August 23, 2023 12:30
@ggwpez ggwpez marked this pull request as draft August 23, 2023 14:24
@juangirini juangirini marked this pull request as ready for review August 23, 2023 14:32
frame/benchmarking/src/lib.rs Show resolved Hide resolved
frame/timestamp/Cargo.toml Outdated Show resolved Hide resolved
@juangirini juangirini requested review from bkchr, liamaharon and a team August 23, 2023 16:12
Copy link
Contributor

@liamaharon liamaharon left a comment

Choose a reason for hiding this comment

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

Looking good, just not sure if a stderr log needs to be adjusted.

Comment on lines +58 to +60
help: consider importing one of these items
|
1 + use frame_benchmarking::__private::traits::PalletInfo;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this suggest

Suggested change
help: consider importing one of these items
|
1 + use frame_benchmarking::__private::traits::PalletInfo;
help: consider importing one of these items
|
1 + use frame_support::traits::PalletInfo;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, do you know if it is possible to somehow tell the compiler to ignore that option?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe @sam0x17 knows

@@ -1,6 +1,7 @@
//! Benchmarking setup for pallet-template
#![cfg(feature = "runtime-benchmarks")]
use super::*;
use sp_std::vec;
Copy link
Member

Choose a reason for hiding this comment

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

This looks wrong? I don't see any usage of the macro in here. Maybe some macro needs this? If yes, it should pull this in automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is expected here https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3434676

[2023-08-24 12:21:48]   error: cannot find macro `vec` in this scope
[2023-08-24 12:21:48]     --> /builds/parity/mirrors/substrate/bin/node-template/pallets/template/src/benchmarking.rs:10:1
[2023-08-24 12:21:48]      |
[2023-08-24 12:21:48]   10 | #[benchmarks]
[2023-08-24 12:21:48]      | ^^^^^^^^^^^^^
[2023-08-24 12:21:48]      |
[2023-08-24 12:21:48]      = help: consider importing one of these items:
[2023-08-24 12:21:48]              crate::benchmarking::__private::vec
[2023-08-24 12:21:48]              frame_benchmarking::__private::vec
[2023-08-24 12:21:48]              scale_info::prelude::vec
[2023-08-24 12:21:48]              sp_std::vec
[2023-08-24 12:21:48]      = note: this error originates in the attribute macro `benchmarks` (in Nightly builds, run with -Z macro-backtrace for more info)

use crate::benchmarks;
use frame_system::Pallet as System;
use sp_runtime::{
traits::{AppVerify, Hash},
RuntimeAppPublic,
};
use sp_std::{vec, vec::Vec};
Copy link
Member

Choose a reason for hiding this comment

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

Here again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is actually required https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3434492

[2023-08-24 12:02:10]   error: cannot find macro `vec` in this scope
[2023-08-24 12:02:10]     --> /builds/parity/mirrors/substrate/frame/benchmarking/src/baseline.rs:93:14
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   93 |             let msg = vec![j, j];
[2023-08-24 12:02:10]      |                       ^^^
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]      = help: consider importing one of these items:
[2023-08-24 12:02:10]              crate::__private::vec
[2023-08-24 12:02:10]              scale_info::prelude::vec
[2023-08-24 12:02:10]              sp_api::vec
[2023-08-24 12:02:10]              sp_std::vec
[2023-08-24 12:02:10] 
[2023-08-24 12:02:10]   error[E0412]: cannot find type `Vec` in this scope
[2023-08-24 12:02:10]     --> /builds/parity/mirrors/substrate/frame/benchmarking/src/baseline.rs:92:21
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   92 |         let msg_and_sigs: Vec<_> = (0..sigs_count).map(|j| {
[2023-08-24 12:02:10]      |                           ^^^ not found in this scope
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   help: consider importing one of these items
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   23 + use crate::__private::Vec;
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   23 + use frame_support::dispatch::Vec;
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   23 + use scale_info::prelude::vec::Vec;
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]   23 + use sp_api::vec::Vec;
[2023-08-24 12:02:10]      |
[2023-08-24 12:02:10]        and 2 other candidates

@juangirini juangirini requested review from bkchr and liamaharon August 24, 2023 11:56
@juangirini
Copy link
Contributor Author

bot merge

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. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D2-breaksapi T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants