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 pub to xcm::v4::PalletInfo #4976

Merged
merged 6 commits into from
Jul 18, 2024
Merged

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Jul 8, 2024

v3 PalletInfo had the fields public, but not v4. Any reason why?
I need the PalletInfo fields public so I can read the values and do some logic based on that at Polimec
@franciscoaguirre

If this could be backported would be highly appreciated 🙏🏻

@JuaniRios JuaniRios requested a review from a team as a code owner July 8, 2024 15:33
Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

This was clearly a misstep, let's backport it

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Needs a prdoc (see directory called prdoc)

@bkchr bkchr added the T6-XCM This PR/Issue is related to XCM. label Jul 8, 2024
@JuaniRios JuaniRios requested a review from acatangiu July 18, 2024 09:19
@JuaniRios
Copy link
Contributor Author

@acatangiu semver check failed because it thinks the change to pub is a major and not a patch. But the pub fields were the original API. Can we override the check?

@acatangiu
Copy link
Contributor

@acatangiu semver check failed because it thinks the change to pub is a major and not a patch. But the pub fields were the original API. Can we override the check?

Don't know honestly. @ggwpez @Morganamilo ?

Changing this to major bump means having to major bump all downstream crates too for no real reason/benefit..

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.

CI seems to be wrong here - this does indeed not look like a major change to me.
Running the semver-check CLI locally confirms this.

prdoc/pr_4976.prdoc Outdated Show resolved Hide resolved
@ggwpez ggwpez enabled auto-merge July 18, 2024 16:46
@ggwpez ggwpez added this pull request to the merge queue Jul 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 18, 2024
@ggwpez ggwpez added this pull request to the merge queue Jul 18, 2024
Merged via the queue into paritytech:master with commit ad9804f Jul 18, 2024
158 of 160 checks passed
franciscoaguirre pushed a commit that referenced this pull request Jul 31, 2024
v3 PalletInfo had the fields public, but not v4. Any reason why?
I need the PalletInfo fields public so I can read the values and do some
logic based on that at Polimec
@franciscoaguirre 

If this could be backported would be highly appreciated 🙏🏻

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
v3 PalletInfo had the fields public, but not v4. Any reason why?
I need the PalletInfo fields public so I can read the values and do some
logic based on that at Polimec
@franciscoaguirre 

If this could be backported would be highly appreciated 🙏🏻

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
EgorPopelyaev pushed a commit that referenced this pull request Aug 15, 2024
Backporting #4976 to
version 1.14.0.

The fields in `PalletInfo` were not `pub` in XCMv4 when they were for
XCMv3.

---------

Co-authored-by: Juan Ignacio Rios <[email protected]>
Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants