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

New rules and labels for the polkadot repo #2

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

the-right-joyce
Copy link
Collaborator

@the-right-joyce the-right-joyce commented Dec 14, 2022

Before merge to-dos:

  • Make sure the release notes scripts are using 'B1-note_worthy' instead of 'B7-runtimenoteworthy' (@coderobe and team)
  • Makes sure the new C* labels are also replaced in the script for creating the release notes (@coderobe and team)
  • Make sure that the CI refers to the D* label rules in this file when there were changes in these paths (@alvicsam):
  • '^runtime/polkadot'
  • '^runtime/kusama'
  • '^primitives/src/'
  • '^runtime/common'

After merge to-dos:

  • Edit issue-bot to use 'J2-unconfirmed' instead of 'Z0-unconfirmed' @alvicsam

Before merge to-dos:
[] Make sure the release notes scripts are using 'B1-note_worthy' instead of 'B1-runtimenoteworthy'
[] Makes sure the new C* labels are also replaced in the script for creating the release notes
[] Make sure that the CI refers to the D* label rules in this file when there were changes in these paths:
- '^runtime/polkadot'
- '^runtime/kusama'
- '^primitives/src/'
- '^runtime/common'

cc: @coderobe
- name: E2-dependencies
description: Pull requests that update a dependency file.
color: "222211"
- name: E3-host_functions

Choose a reason for hiding this comment

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

What's the point of this? it seems like a pure subset of the next item.

description: This PR/Issue is related to the topic “runtime”.
color: F5FCE6
- name: T2-API
description: This PR/Issue is related to APIs.

Choose a reason for hiding this comment

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

Which API? RPC api? runtime api? rust crate api?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can extend the options here, right now it's very generic

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Looking forward to similar changes in Substrate!

description: PR introduces code that does a one-way migration of the database.
color: "112233"
- name: E2-dependencies
description: Pull requests that update a dependency file.
Copy link
Member

Choose a reason for hiding this comment

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

Could we automatically enforce this through the CI by checking for changes in the Cargo.lock files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think so but I need more info about the logic how it should be checked/triggered. It seems that it's going to be a custom job.

Copy link
Member

Choose a reason for hiding this comment

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

Me neither. @the-right-joyce could you please put it into a new issue so we can keep track of it? https://github.com/paritytech/ci_cd/issues

ruled_labels/specs_polkadot.yaml Outdated Show resolved Hide resolved
ruled_labels/specs_polkadot.yaml Outdated Show resolved Hide resolved
ruled_labels/specs_polkadot.yaml Show resolved Hide resolved
- name: J5-wont_fix
description: "Issue is in principle valid, but this project will not address it. Closer should explain why."
color: FB3701
- name: J6-invalid
Copy link
Member

Choose a reason for hiding this comment

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

Not sure where it fits best: But a reserved label similar to the mentor label would be nice - to indicate that an issue should not be picked up by anyone unless approved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and who should be the one to approve this issue?

Copy link
Member

Choose a reason for hiding this comment

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

Just someone from Parity in general. That person should then know why the reserved label was on there.
Currently we often have the case that external contributors "snatch" issues that we had reserved for someone new as on-boarding task.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

do we really want to do this as an open source project?

Copy link
Member

Choose a reason for hiding this comment

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

My problem currently is that some advanced external devs are swooping up easy issues that we ought to keep for new-comers as entry to Substrate.
Even that label won't prevent them from working on it, but hopefully make them ask first.

implemented all changes requested
added rule that requires T0 and T1
@the-right-joyce the-right-joyce requested review from alvicsam and kianenigma and removed request for kianenigma January 20, 2023 13:26
@the-right-joyce the-right-joyce merged commit 5e4692c into main Jan 20, 2023
@the-right-joyce the-right-joyce deleted the the-right-joyce-patch-1 branch January 20, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants