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

core-fellowship: promote dev-dependencies to regular dependencies#13665

Merged
altaua merged 2 commits intomasterfrom
mira/core-fellowship-runtime-benchmarks
Mar 21, 2023
Merged

core-fellowship: promote dev-dependencies to regular dependencies#13665
altaua merged 2 commits intomasterfrom
mira/core-fellowship-runtime-benchmarks

Conversation

@altaua
Copy link
Copy Markdown
Contributor

@altaua altaua commented Mar 21, 2023

These pallets are not just required for tests, but also for the runtime-benchmarks feature.

CI is currently failing on master due to this.

@altaua altaua added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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. labels Mar 21, 2023
@altaua altaua self-assigned this Mar 21, 2023
@altaua altaua added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Mar 21, 2023
ggwpez
ggwpez reviewed Mar 21, 2023
View reviewed changes
@altaua altaua force-pushed the mira/core-fellowship-runtime-benchmarks branch from fa53c64 to 43f0c89 Compare March 21, 2023 12:29
@altaua
Copy link
Copy Markdown
Contributor Author

altaua commented Mar 21, 2023

bot merge

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot bot commented Mar 21, 2023

Waiting for commit status.

@altaua
Copy link
Copy Markdown
Contributor Author

altaua commented Mar 21, 2023

bot merge cancel

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot bot commented Mar 21, 2023

Merge cancelled.

bkchr
bkchr suggested changes Mar 21, 2023
View reviewed changes
Copy link
Copy Markdown
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.

This is wrong.

The proper solution is this: paritytech/releng-scripts#37

@altaua
Copy link
Copy Markdown
Contributor Author

altaua commented Mar 21, 2023

This is wrong.

The proper solution is this: paritytech/releng-scripts#37

In this particular case, an optional regular dependency actually seems more accurate to me than a dev-dependency -- as far as I can see, those pallets aren't required by the tests, just by the runtime benchmarks.

bkchr
bkchr approved these changes Mar 21, 2023
View reviewed changes
@altaua altaua force-pushed the mira/core-fellowship-runtime-benchmarks branch from 43f0c89 to 594560b Compare March 21, 2023 15:05
@altaua
Copy link
Copy Markdown
Contributor Author

altaua commented Mar 21, 2023

bot merge

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot bot commented Mar 21, 2023

Waiting for commit status.

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot bot commented Mar 21, 2023

Merge cancelled due to error. Error: Statuses failed for 594560b

@altaua altaua force-pushed the mira/core-fellowship-runtime-benchmarks branch from 594560b to 615399b Compare March 21, 2023 15:15
@ggwpez
Update lockfile
89a6364 
The CI needs this.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@altaua altaua merged commit 6f79a5a into master Mar 21, 2023
@altaua altaua deleted the mira/core-fellowship-runtime-benchmarks branch March 21, 2023 17:17
@bkchr
Copy link
Copy Markdown
Member

bkchr commented Mar 21, 2023

@altaua did you used rebase merging?

@ggwpez
Copy link
Copy Markdown
Member

ggwpez commented Mar 22, 2023

Master branch:
Screenshot from 2023-03-22 12-12-22

How can this happen, since it is not enabled in this repo?
Screenshot from 2023-03-22 12-11-40

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Mar 22, 2023

How can this happen, since it is not enabled in this repo?

I disabled it afterwards :P

@ggwpez
Copy link
Copy Markdown
Member

ggwpez commented Mar 22, 2023 •
edited
Loading

How can this happen, since it is not enabled in this repo?

I disabled it afterwards :P

Okay 👍
Does that also reject merge commits over SSH directly? Or is this setting only for the Github Website?

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Mar 22, 2023

I have no idea :P

@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

5 more reviewers

@pgherveou pgherveou pgherveou left review comments

@ggwpez ggwpez ggwpez left review comments

@bkchr bkchr bkchr approved these changes

@acatangiu acatangiu acatangiu approved these changes

@lexnv lexnv lexnv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@altaua altaua

Labels

A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@altaua @bkchr @ggwpez @pgherveou @acatangiu @lexnv