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

Submit xmc-tools-follow-up m1 #1154

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Submit xmc-tools-follow-up m1 #1154

merged 4 commits into from
Apr 8, 2024

Conversation

gmajor-encrypt
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, invoices must be submitted and payments will be transferred to the Polkadot AssetHub and/or fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#2205

@PieWol PieWol self-assigned this Mar 19, 2024
@PieWol PieWol added the last milestone The team delivered the last milestone of the project label Mar 20, 2024
@PieWol
Copy link
Member

PieWol commented Mar 20, 2024

Hey @gmajor-encrypt ,
thank you for submitting the delivery. I will notify you once my evaluation is completed.

@PieWol
Copy link
Member

PieWol commented Mar 24, 2024

Hi @gmajor-encrypt ,
my evaluation is finished with some changes requested. Looking forward to your comments on this and the changes you will make. Please notify me as soon as you have updates to share.

@gmajor-encrypt
Copy link
Contributor Author

@PieWol Thanks for your evaluation. I will make change as soon.

@gmajor-encrypt
Copy link
Contributor Author

@PieWol I have update the code. Please check again.

Xcm V3 struct not as strictly typed as the rust implementation

Since xcm-tools does not involve the need to construct xcm messages. Therefore, dynamic types are used here, which are only useful when parse messages.

@PieWol
Copy link
Member

PieWol commented Apr 2, 2024

Hey @gmajor-encrypt,
thanks for the updates. Changes to the documentation are looking good. Have you made sure that the tests are working? After pulling your changes, building again and starting the tests I have the issue that the tests are still not passing.

Please let me know once you fixed the tests in Docker. I have pulled the changes and re ran the commands to build and test with Docker. If I am missing a step please let me know as well. I have updated my evaluation so it reflects the current issues and testing output.

Thanks :)

@gmajor-encrypt
Copy link
Contributor Author

gmajor-encrypt commented Apr 3, 2024

@PieWol Thanks for your evaluation. I think the lack of test success in docker is because there is no re-docker build. You can try it after building. docker build -t xcm-tools . && docker run -it --rm xcm-tools

@PieWol
Copy link
Member

PieWol commented Apr 8, 2024

I'm really confused as I was pretty sure that I have built it again after pulling the latest changes. Yet for some reason after building again it is now working. Thanks for your patience.

I'll have my evaluation updated so that it is now accepted.

Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

I just merged the evaluation. Your milestone is hereby accepted. Congrats, @gmajor-encrypt.

@semuelle semuelle merged commit b1039a6 into w3f:master Apr 8, 2024
3 checks passed
@RouvenP
Copy link

RouvenP commented Apr 22, 2024

hi @gmajor-encrypt we sent the payment last Friday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last milestone The team delivered the last milestone of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants