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

feat: coretime implementation #1558

Merged
merged 26 commits into from
Jan 7, 2025
Merged

feat: coretime implementation #1558

merged 26 commits into from
Jan 7, 2025

Conversation

filvecchiato
Copy link
Contributor

@filvecchiato filvecchiato commented Nov 29, 2024 •
edited
Loading

New Feature: Implements the coretime endpoints for relay chain and for coretime chain.

It includes basic endpoints for single information record recovery, and more advanced endpoints with precomputed calculations and information.

@filvecchiato filvecchiato force-pushed the filippo_coretime_implementation branch from 744895d to 6de3395 Compare November 29, 2024 15:52
@filvecchiato filvecchiato changed the title Filippo coretime implementation Feat: coretime implementation Nov 29, 2024
@filvecchiato filvecchiato force-pushed the filippo_coretime_implementation branch from 4efc743 to f6e8a77 Compare December 9, 2024 15:15
@filvecchiato filvecchiato changed the title Feat: coretime implementation feat: coretime implementation Dec 9, 2024
@filvecchiato filvecchiato force-pushed the filippo_coretime_implementation branch from 9ef9d68 to e8a0808 Compare December 19, 2024 15:59
@filvecchiato filvecchiato force-pushed the filippo_coretime_implementation branch from 142039d to 4dcdf17 Compare December 20, 2024 11:12
@filvecchiato filvecchiato marked this pull request as ready for review December 20, 2024 11:13
@filvecchiato filvecchiato requested a review from a team as a code owner December 20, 2024 11:13
TarikGul
TarikGul requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Overall amazing PR! I have a few nits:

  1. The typecasting can be removed. AFAIU the types are correctly passed down from PJS so you should be good.

  2. For any imported types we can use import type.

Other than that the PR looks really great!

@filvecchiato
Copy link
Contributor Author

  1. The typecasting can be removed. AFAIU the types are correctly passed down from PJS so you should be good.

I removed most of typecasting (nice catch, not sure I was having issues before), I had to leave typecasting for coreDescriptors and paraLifecycles though, we can doublecheck if there is something wrong on my end though

Other than that the PR looks really great!

thanks for checking it out!

@filvecchiato filvecchiato linked an issue Dec 23, 2024 that may be closed by this pull request
Add support for Coretime #1413
Closed
TarikGul
TarikGul approved these changes Dec 24, 2024
View reviewed changes
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

lgtm!

Imod7
Imod7 requested changes Jan 1, 2025
View reviewed changes
Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Some minor comments and a question. Looks great! 🎉

docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
src/controllers/coretime/CoretimeChainController.ts Outdated Show resolved Hide resolved
src/chains-config/polkadotControllers.ts Outdated Show resolved Hide resolved
src/controllers/coretime/CoretimeGenericController.ts Outdated Show resolved Hide resolved
src/services/coretime/CoretimeService.ts Outdated Show resolved Hide resolved
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
@filvecchiato
Copy link
Contributor Author

Some minor comments and a question. Looks great! 🎉

Nice catches, thanks! On it

@filvecchiato filvecchiato requested a review from Imod7 January 6, 2025 12:02
Imod7
Imod7 approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Thanks for updating! All Good! ✅

@filvecchiato filvecchiato merged commit 3a92196 into master Jan 7, 2025
15 checks passed
@filvecchiato filvecchiato deleted the filippo_coretime_implementation branch January 7, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TarikGul TarikGul TarikGul approved these changes

@Imod7 Imod7 Imod7 approved these changes

@IkerAlus IkerAlus Awaiting requested review from IkerAlus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Coretime
3 participants
@filvecchiato @Imod7 @TarikGul