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

Add check runtimes GH Workflow #2252

Merged
merged 37 commits into from
Dec 13, 2023
Merged

Add check runtimes GH Workflow #2252

merged 37 commits into from
Dec 13, 2023

Conversation

chevdor
Copy link
Contributor

@chevdor chevdor commented Nov 9, 2023

This PR introduces:

  • a new script
  • a new GH Workflow
  • runtime reference spec files for rococo and westend

It brings a mechanism to check that part(s) of the runtimes' metadata that should not change over time, actually did not change over time.

Ideally, the GHW should trigger when a release is edited but GH seem to have an issue that prevents the trigger from working. This is why the check has been implemented as workflow_dispatch for a start.

The workflow_dispatch requires a release_id that can be found using:

curl -s -H "Authorization: Bearer ${GITHUB_TOKEN}" \
   https://api.github.com/repos/paritytech/polkadot-sdk/releases | \
   jq '.[] | { name: .name, id: .id }'

as documented in the workflow.

A sample run can be seen here.

@chevdor chevdor added the R0-silent Changes should not be mentioned in any release notes label Nov 9, 2023
@chevdor chevdor requested review from a team as code owners November 9, 2023 13:24
@@ -0,0 +1,17 @@
{
"pallets": {
"1": {
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 go by pallet name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this first version, I kept things simple and I am not interpreting the specs. This is why they must match 100% the format of the actuall metadata.

That could be improvement in the future but I would rather get started simple to have this check in place asap.

The benefit of the current option as well is that we can pretty much use the current metadata as spec, that would be may too strict but it works as a starting point.


"2": {
"constants": {
"MinimumPeriod": {
Copy link
Member

Choose a reason for hiding this comment

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

Does it detect typos in the constant names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the spec contains:

 "2": {
        "constants": {
          "foo": { ... }

and the metadata :

 "2": {
        "constants": {
          "foobar": { ... }

It will not detect a typo but it will not find foo and fail.

@@ -0,0 +1,124 @@
#!/usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should put this into the pipeline-scripts or releng-scripts repo, since it will be useful in the runtimes repo as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, I would create a followup PR for the fellowship runtimes.
I think having this script in the polkadot-sdk repo does also make sense since downstream repos will already likely depend on the polkadot-sdk repo.

Copy link
Contributor Author

@chevdor chevdor left a comment

Choose a reason for hiding this comment

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

Added comments

@@ -0,0 +1,17 @@
{
"pallets": {
"1": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this first version, I kept things simple and I am not interpreting the specs. This is why they must match 100% the format of the actuall metadata.

That could be improvement in the future but I would rather get started simple to have this check in place asap.

The benefit of the current option as well is that we can pretty much use the current metadata as spec, that would be may too strict but it works as a starting point.


"2": {
"constants": {
"MinimumPeriod": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the spec contains:

 "2": {
        "constants": {
          "foo": { ... }

and the metadata :

 "2": {
        "constants": {
          "foobar": { ... }

It will not detect a typo but it will not find foo and fail.

@@ -0,0 +1,124 @@
#!/usr/bin/env python3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, I would create a followup PR for the fellowship runtimes.
I think having this script in the polkadot-sdk repo does also make sense since downstream repos will already likely depend on the polkadot-sdk repo.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team November 28, 2023 10:51
@chevdor chevdor requested a review from ggwpez December 13, 2023 08:46
@chevdor chevdor enabled auto-merge (squash) December 13, 2023 09:04
@chevdor chevdor merged commit 3c5fcbe into master Dec 13, 2023
@chevdor chevdor deleted the wk-231108-check-runtimes branch December 13, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants