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.

Sign up for GitHub

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

Jump to bottom

Remove schedule native transfer #377

Merged
merged 22 commits into from
Jul 7, 2023
Merged

Remove schedule native transfer #377

merged 22 commits into from
Jul 7, 2023

Conversation

v9n
Copy link
Member

@v9n v9n commented Jun 29, 2023

This task is obsolute by schedule_dynamic_dispatch_tasks and passing the Balance extrinsic

@v9n v9n changed the base branch from master to remove-schedule_notify June 29, 2023 10:58
@v9n v9n changed the title Remove schedule native transfer [wip]Remove schedule native transfer Jun 29, 2023
@v9n v9n marked this pull request as draft June 29, 2023 10:58
@v9n v9n added the WIP work in progress, not ready for review label Jun 29, 2023
@v9n v9n self-assigned this Jun 29, 2023
@v9n v9n force-pushed the remove-schedule-native-transfer branch from 801551a to 5c2ef91 Compare June 30, 2023 10:02
@v9n v9n changed the title [wip]Remove schedule native transfer Remove schedule native transfer Jun 30, 2023
@v9n v9n marked this pull request as ready for review June 30, 2023 10:22
@v9n v9n requested review from chrisli30 and imstar15 and removed request for chrisli30 June 30, 2023 10:22
@v9n v9n removed the WIP work in progress, not ready for review label Jun 30, 2023
chrisli30
chrisli30 approved these changes Jun 30, 2023
View reviewed changes
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

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

LGTM

pallets/automation-time/src/tests.rs Outdated Show resolved Hide resolved
imstar15
imstar15 requested changes Jul 1, 2023
View reviewed changes
Copy link
Member

@imstar15 imstar15 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

image

Please run the commands and fix to pass the two checks.

rustfmt

cargo fmt -- --check

Test

cargo build --workspace --all-features
cargo test --workspace --all-features

@v9n v9n force-pushed the remove-schedule-native-transfer branch from b4acc76 to 1deb99b Compare July 4, 2023 05:40
@ciphervinh @v9n
remove schedule_native_transfer_tasks
9c14f59 
the code is removed and a test to show case how we use
Balances::Call::transfer to initiate this
@v9n v9n force-pushed the remove-schedule-native-transfer branch from 6b0e2bf to 9c14f59 Compare July 4, 2023 06:13
Base automatically changed from remove-schedule_notify to update-readme-for-mangata-node-compile July 5, 2023 05:49
@v9n v9n force-pushed the update-readme-for-mangata-node-compile branch from d0a2c78 to 9b00a75 Compare July 5, 2023 16:05
@v9n v9n changed the base branch from update-readme-for-mangata-node-compile to remove-schedule_notify July 5, 2023 16:18
@v9n
Copy link
Member Author

v9n commented Jul 5, 2023

@imstar15 Hi Charles, this PR is fixed now and I added a new tests as well to test schedule transfer using dynamic dispatch

chrisli30
chrisli30 reviewed Jul 5, 2023
View reviewed changes
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

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

Looks good to me overall, but I have got several comments.

pallets/automation-time/src/tests.rs Outdated Show resolved Hide resolved
pallets/automation-time/src/tests.rs Outdated Show resolved Hide resolved
pallets/automation-time/src/mock.rs Show resolved Hide resolved
Base automatically changed from remove-schedule_notify to master July 6, 2023 16:25
imstar15
imstar15 approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@imstar15 imstar15 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@v9n v9n merged commit 7b3d4e4 into master Jul 7, 2023
2 checks passed
@chrisli30 chrisli30 deleted the remove-schedule-native-transfer branch July 7, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisli30 chrisli30 chrisli30 approved these changes

@imstar15 imstar15 imstar15 approved these changes

Assignees

@v9n v9n

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@v9n @chrisli30 @imstar15 @ciphervinh