-
Notifications
You must be signed in to change notification settings - Fork 9
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
chore: comment out ismp #414
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #414 +/- ##
==========================================
- Coverage 68.41% 68.37% -0.05%
==========================================
Files 70 69 -1
Lines 11838 11845 +7
Branches 11838 11845 +7
==========================================
Hits 8099 8099
- Misses 3482 3489 +7
Partials 257 257
|
before approving this, I think it's worthwhile to see if we can revert back to our ISMP fork and update that repo as well. In the past it's been smooth with only PSVM being required. See this commit: r0gue-io/ismp@46862a0 IMPORTANT NOTE:The ISMP repo will NOT build standalone. You will verify its compilation through building pop-node with the ISMP modules pointing to your new ISMP branch. This is because we are only using the ISMP pallet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If everything related to ISMP removed, why don't we fork main
branch to ismp
, create a PR and then remove everything related to ismp in main
. This avoid leaving all the commented-out code in everywhere in the file.
Edit: Saw you removed review requests, I'll leave my comments here anyway.
- name: Check Build with ISMP | ||
run: | | ||
cargo check --release --locked --features=ismp,runtime-benchmarks,try-runtime | ||
# check-ismp: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this has been merged into main
, it's worth leaving clear TODOs for the commented-out code.
In order to uplift pop to stable 2412 the ismp related code will be commented out. This is because the ismp dependencies are currently at 2407 and there is no eta when they will do those upgrades. In the meantime we can proceed with the upgrades and relevant work depending on this.
As for the messaging feature that is in the pipeline, I created a branch from main:
feat/messaging
. This branch is on v1.14 with ismp still in the devnet runtime.Before reviewing, see latest commit for something that was required to make
zepter
happy.