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

Re-add deprecated --execution arg on benchmark pallet #14567

Merged
merged 4 commits into from
Jul 13, 2023
Merged

Re-add deprecated --execution arg on benchmark pallet #14567

merged 4 commits into from
Jul 13, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jul 12, 2023 •
edited
Loading

Changes:

  • Re-add the --execution arg on command benchmark pallet that was removed in #14387. It is a No-OP and prints ⚠️ Argument --execution is deprecated. Its value of wasm will be ignore now upon usage.
  • Add DeferGuard::new as convenience function.
  • Extend benchmark * cmd tests.

ggwpez added 3 commits July 12, 2023 16:55
@ggwpez
Add DeferGuard::new
9166c59 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Re-add deprecated 'execution' arg to benchmark pallet cmd.
8ee001c 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Extend tests
3dcbb94 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added 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 T1-runtime This PR/Issue is related to the topic “runtime”. labels Jul 12, 2023
@ggwpez ggwpez self-assigned this Jul 12, 2023
@bkchr
Copy link
Member

bkchr commented Jul 12, 2023 •
edited
Loading

Why bring it back?

I mean yes, I should not have removed it directly, but now this happened. This is also more a dev centric command and not really used by "users".

@ggwpez
Copy link
Member Author

ggwpez commented Jul 12, 2023

Why bring it back?

I mean yes, I should not have removed it directly, but now this happened. This is also more a dev centric command and not really used by "users".

Its probably used in automation (also by downstream teams). So I think having some grace period is some nicer experience.
But its surely not really necessary, so I can also close again 🤷‍♂️

@bkchr
Copy link
Member

bkchr commented Jul 12, 2023

Why bring it back?
I mean yes, I should not have removed it directly, but now this happened. This is also more a dev centric command and not really used by "users".

Its probably used in automation (also by downstream teams). So I think having some grace period is some nicer experience. But its surely not really necessary, so I can also close again man_shrugging

Yeah, IDK. Then let's keep it, but we don't need this in all the tests.

@ggwpez
Remove from tests
488ba9b 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Jul 13, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit f2a5ca0 into master Jul 13, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-warn-pallet-execution-arg branch July 13, 2023 17:41
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@ggwpez @nathanwhit
Re-add deprecated --execution arg on benchmark pallet (paritytech…
cb17c08 
…#14567)

* Add DeferGuard::new

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Re-add deprecated 'execution' arg to benchmark pallet cmd.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Extend tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove from tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Ank4n pushed a commit that referenced this pull request Jul 22, 2023
@ggwpez @Ank4n
Re-add deprecated --execution arg on benchmark pallet (#14567)
112824c 
* Add DeferGuard::new

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Re-add deprecated 'execution' arg to benchmark pallet cmd.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Extend tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove from tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@JesseAbram JesseAbram mentioned this pull request Oct 26, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@lexnv lexnv lexnv approved these changes

Assignees

@ggwpez ggwpez

Labels
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 T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Runtime / FRAME
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ggwpez @bkchr @lexnv