unified all tests into a concluding test #2
Conversation
Test was singular, it should be plural
The matrix jobs are already depended by other jobs. We can simply remove their dependency
It looks a bit better than simply adding a print line statement
Let's unify everything at once
Added a shorter name to make it easier to understand
|
Warning Rate Limit Exceeded@Bullrich has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 44 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates introduce enhancements and deprecations in the workflow and codebase, focusing on migration and test verification, XCM version handling, and constant deprecation in favor of updated variants. These changes streamline CI processes, refine integration tests, and update constants for better code clarity and future compatibility. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (9)
- .github/workflows/check-migrations.yml (1 hunks)
- .github/workflows/test.yml (1 hunks)
- CHANGELOG.md (1 hunks)
- integration-tests/emulated/assets/asset-hub-kusama/src/tests/send.rs (1 hunks)
- integration-tests/emulated/assets/asset-hub-kusama/src/tests/set_xcm_versions.rs (1 hunks)
- relay/polkadot/constants/src/lib.rs (1 hunks)
- relay/polkadot/src/xcm_config.rs (2 hunks)
- system-parachains/collectives/collectives-polkadot/build.rs (1 hunks)
- system-parachains/collectives/collectives-polkadot/src/xcm_config.rs (2 hunks)
Additional comments: 9
integration-tests/emulated/assets/asset-hub-kusama/src/tests/send.rs (1)
- 87-87: The update to the numeric value in the test seems to reflect an adjustment in expected weights or fees. Ensure this change aligns with the latest expectations for the test's functionality.
.github/workflows/check-migrations.yml (1)
- 115-121: The addition of the
confirmMigrationsPassedjob to the workflow is correctly implemented and aligns with the PR's objectives to enhance the project's CI/CD pipeline by ensuring migrations conclusively pass before proceeding.relay/polkadot/constants/src/lib.rs (1)
- 112-113: The deprecation of the
TREASURER_INDEXconstant is correctly implemented, with a clear message guiding developers towards usingxcm::latest::BodyId::Treasuryinstead. This change aligns with the project's objectives to standardize constants within the XCM configurations..github/workflows/test.yml (1)
- 208-217: The addition of the
confirmTestPassedjob to the workflow is correctly implemented and aligns with the PR's objectives to enhance the project's CI/CD pipeline by ensuring tests conclusively pass before proceeding.CHANGELOG.md (1)
- 22-22: The documentation of the deprecation and replacement of the
xcm::body::TREASURER_INDEXconstant in the CHANGELOG.md is clear and follows the standard changelog format. Including the link to the pull request is a good practice for traceability.relay/polkadot/src/xcm_config.rs (2)
- 32-32: The reorganization of imports and the adjustment of constants for
FellowshipAdminBodyIdandTreasurerBodyIdare correctly implemented. It's important to ensure that these changes are reflected wherever these constants are used throughout the project to maintain consistency.Verification successful
The constants
FellowshipAdminBodyIdandTreasurerBodyIdappear to be used consistently throughout the project, as indicated by their presence in multiple files across different directories. This suggests that the changes have been correctly implemented and propagated. However, given the complexity and specificity of their usage contexts, it's recommended to manually review these instances to ensure their correctness and consistency fully.* 239-239: The update to use `BodyId::Treasury` for `TreasurerBodyId` aligns with the PR's objective to standardize and update constants related to XCM configurations. Ensure that this change is consistently applied across all relevant parts of the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the updated constants are correctly used throughout the project. rg --type rust 'FellowshipAdminBodyId|TreasurerBodyId'Length of output: 2245
Verification successful
The usage of
TreasurerBodyIdasBodyId::Treasuryacross the project appears consistent and aligns with the objective to standardize and update constants related to XCM configurations. The constant is defined and used in a manner that suggests a coherent application across different parts of the codebase, including both definitions and functional contexts. Based on the provided evidence, the update seems to have been consistently applied across all relevant parts of the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Confirm the consistent use of TreasurerBodyId across the project. rg --type rust 'TreasurerBodyId'Length of output: 1115
system-parachains/collectives/collectives-polkadot/src/xcm_config.rs (2)
- 33-33: The removal of the
xcm_constantsimport and the subsequent change to useBodyId::Treasurydirectly is a positive change towards standardizing constants within the project's XCM configurations. This aligns with the PR's objective of updating deprecated elements to align with newer standards or requirements. However, ensure that all references to the deprecatedxcm::body::TREASURER_INDEXconstant elsewhere in the codebase have been updated accordingly to prevent any unresolved references or compilation errors.Verification successful
The search for references to the deprecated constant
xcm::body::TREASURER_INDEXdid not yield any results, indicating that it has likely been successfully removed or replaced throughout the codebase. This supports the initial assessment that the changes align with the project's goal of updating and standardizing constants within the XCM configurations. However, it's recommended to conduct a comprehensive review of the entire codebase to ensure all necessary updates have been made and to verify the integration and functionality of these changes in the broader context.* 61-61: The update to `TreasurerBodyId` to use `BodyId::Treasury` directly is a clear and straightforward change that improves the clarity and maintainability of the code by relying on the more descriptive and standardized `BodyId::Treasury` variant. This change is consistent with the PR's objectives and does not introduce any apparent issues. However, it's important to verify that this change is reflected wherever `TreasurerBodyId` is utilized to ensure consistency across the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the deprecated constant. rg --type rust 'xcm::body::TREASURER_INDEX'Length of output: 43
Verification successful
The search results indicate that the update to
TreasurerBodyIdto useBodyId::Treasuryis consistently applied across the codebase, with references found in multiple files within both thesystem-parachains/collectives/collectives-polkadotandrelay/polkadotdirectories. This suggests that the change is correctly reflected whereverTreasurerBodyIdis utilized, aligning with the PR's objectives for clarity and maintainability.However, given the nature of code review and the limitations of not being able to inspect the exact context of each usage, it's recommended to perform a manual review to ensure that the change aligns with the intended functionality in all cases and that there are no hidden issues.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any usage of `TreasurerBodyId` to ensure consistency. rg --type rust 'TreasurerBodyId'Length of output: 1115
| @@ -16,8 +16,7 @@ | |||
| #[cfg(feature = "std")] | |||
| fn main() { | |||
| substrate_wasm_builder::WasmBuilder::new() | |||
| .with_current_project() | |||
| .export_heap_base() | |||
| .with_curree() | |||
There was a problem hiding this comment.
The method with_curree() seems to be incorrect or a typo. Please verify the intended functionality. If you meant to use a method from the substrate_wasm_builder crate to configure the Wasm build process, it's possible that the correct method name might be different.
| <AssetHubKusama as Chain>::RuntimeCall::PolkadotXcm(pallet_xcm::Call::< | ||
| <AssetHubKusama as Chain>::Runtime, | ||
| >::force_xcm_version { | ||
| fn system_para_sersion { |
There was a problem hiding this comment.
The function name system_para_sersion seems to be a typo. Consider renaming it to system_para_version or a more descriptive name that accurately reflects the test's purpose.
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
a815839 to
cb2f032
Compare
Summary by CodeRabbit
New Features
Documentation
xcm::body::TREASURER_INDEXconstant.Refactor