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

Remove hardcoded code for root endpoint. #459

Merged
merged 6 commits into from
May 23, 2024

Conversation

mariacarmina
Copy link
Member

@mariacarmina mariacarmina commented May 20, 2024

Fixes #435 .

Changes proposed in this PR:

  • complete the endpoints for /
  • fix chain ids list
  • tested on remote VM

@mariacarmina mariacarmina self-assigned this May 20, 2024
@mariacarmina mariacarmina marked this pull request as ready for review May 21, 2024 12:14
@mariacarmina
Copy link
Member Author

Response from root endpoint now:

Screenshot 2024-05-21 at 15 41 03

@mariacarmina mariacarmina changed the title Remove hardcoded code for chain ids retrieval. Remove hardcoded code for root endpoint. May 21, 2024
src/OceanNode.ts Outdated Show resolved Hide resolved
@mariacarmina mariacarmina requested a review from paulo-ocean May 22, 2024 12:28
@jamiehewitt15
Copy link
Member

This is good, but it would be even better if the route endpoints are also dynamically generated, rather than hardcoded in a json. As you've seen that list tends to be ignored and it will probably become out of date again soon. It would be nice if we don't have to maintain it.

@paulo-ocean
Copy link
Contributor

paulo-ocean commented May 23, 2024

This is good, but it would be even better if the route endpoints are also dynamically generated, rather than hardcoded in a json. As you've seen that list tends to be ignored and it will probably become out of date again soon. It would be nice if we don't have to maintain it.

I agree, this will get outdated again probably, but probably better do it on another PR. i can create another PR for this

Copy link
Contributor

@paulo-ocean paulo-ocean left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@jamiehewitt15 jamiehewitt15 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, the endpoints will be handled in another PR

@mariacarmina mariacarmina merged commit c2916cc into develop May 23, 2024
6 checks passed
@mariacarmina mariacarmina deleted the refactor-root-endpoint branch May 23, 2024 14:13
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