Skip to content

TCDT Milestone 2 #1242

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

khalidzahra
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: Private application. Project name: tcdt

@PieWol PieWol self-assigned this Jan 23, 2025
@github-actions github-actions bot added the stale label Feb 7, 2025
@PieWol PieWol removed the stale label Feb 7, 2025
@github-actions github-actions bot added the stale label Feb 22, 2025
@semuelle semuelle removed the stale label Feb 24, 2025
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.

Hi @khalidzahra, the tcdt-milestone-1.md file is already merged (#1241). Can you remove it from this PR?

Also, sorry for the delay. @PieWol is currently out, but will soon return to evaluating.

@khalidzahra
Copy link
Contributor Author

Hey @semuelle, no worries! I removed the tcdt-milestone-1.md file as you requested :)

@PieWol
Copy link
Member

PieWol commented Mar 5, 2025

Hey @khalidzahra ,
sorry for the delay. I was out last week. Could you please adjust the links for the protocol deliverables so that they point to the relevant code? I'd argue that would be more expressive than the images within the readme. What do you think?

@PieWol
Copy link
Member

PieWol commented Mar 5, 2025

For the testing guide deliverable it would be nice if you could create an easy to follow step by step guide that demonstrates what was achieved in this milestone.

@khalidzahra
Copy link
Contributor Author

Hey @PieWol, thank you for your valuable feedback and sorry for the delayed response. I have created testing guides (linked at the bottom of the main README.md of the project) for the milestone's deliverables, and I went ahead and added unit tests that can simply be run to verify their functionality.

Regarding the protocols, some parts are designed to be offloaded to other blockchains that want to use the project, so there wouldn't be any implementation present in the repository to point to. Furthermore, the revocation protocol will be fully implemented in the third milestone, so I will update the links to point to it then. I have however added links to the relevant code snippets that contribute to each protocol under their respective sections within the README.md.

Let me know if anything is missing, and if these changes are what you expected!

@github-actions github-actions bot added the stale label Mar 22, 2025
@PieWol PieWol removed the stale label Mar 25, 2025
@github-actions github-actions bot added the stale label Apr 9, 2025
@khalidzahra
Copy link
Contributor Author

Hey @PieWol, sorry for the delay! I have added scripts to help in launching the architecture and to also set it up with some data for testing purposes. I updated the testing guides to include how the architecture can be launched as well. Let me know if this is what you were looking for!

@semuelle
Copy link
Member

Hey @khalidzahra, sorry for the wait. I will take over the evaluation for Piet while he's out. I'll get back to you with feedback as soon as possible.

@semuelle semuelle removed the stale label Apr 14, 2025
@semuelle semuelle self-assigned this Apr 14, 2025
@PieWol
Copy link
Member

PieWol commented Apr 28, 2025

Hey @khalidzahra ,
thanks for the updates. I'm finally back from vacation and took a look already. Sadly the unit tests are currently failing. I updated my evaluation accordingly. Let me know whats wrong. Once this is resolved I'll proceed with the actual testing with a deployed node.

@khalidzahra
Copy link
Contributor Author

Hey @PieWol, thanks for catching that. There was some mismatch between module names which was causing the tests to fail, but I have fixed it and everything should run smoothly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants