chore: clarify api folders #3637
Conversation
…accessor to node interface + make everything compile after renaming
NagyZoltanPeter
requested review from
Ivansete-status,
Copilot,
darshankabariya,
fcecin and
fryorcraken
There was a problem hiding this comment.
Pull Request Overview
This PR performs a major refactoring by renaming the waku_api module to waku_rest, and reorganizing the internal API structure. The changes include:
- Renaming
waku_api→waku_rest - Renaming
node/api→node/kernel_api - Moving REST-related code into
waku_rest/endpoint/ - Updating import statements across the entire codebase
- Minor comment updates (e.g., "Public API types" → "API types")
Reviewed Changes
Copilot reviewed 48 out of 89 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
waku/waku_rest/ (multiple files) |
New REST endpoint implementations moved from waku_api |
waku/node/kernel_api/ (multiple files) |
Core node API functionality moved from node/api |
waku/waku_store_legacy/common.nim |
Updated comment from "Public API types" to "API types" |
waku/waku_store/common.nim |
Updated comment from "Public API types" to "API types" |
| Test files | Updated imports to reflect new module structure |
| Application files | Updated imports to reflect new module structure |
waku/factory/waku_conf.nim |
Fixed trailing whitespace in comment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
You can find the image built from this PR at Built from ef2af46 |
Ivansete-status
left a comment
Ivansete-status
left a comment
There was a problem hiding this comment.
LGTM! Thanks for it! 💯
Just added some suggestions that I hope you find interesting ( not blocking .)
| results, | ||
| libp2p/peerid | ||
|
|
||
| from std/sugar import `=>` |
There was a problem hiding this comment.
Interesting! What is it for?
There was a problem hiding this comment.
Yeah, LTP was not compiling as I found during working on this PR. Some former refactor moved things.
This operator was needed here, and I did not want to import all from sugar, which is an extensive module anyway.
|
|
||
| import | ||
| ../wakunode2/cli_args, | ||
| ../../tools/confutils/cli_args, |
There was a problem hiding this comment.
Everything can be referenced from the repo's root folder. We can adopt that style as much as possible
| ../../tools/confutils/cli_args, | |
| tools/confutils/cli_args, |
apps/chat2bridge/chat2bridge.nim
Outdated
| # @TODO confutils.nim(775, 17) Error: can raise an unlisted exception: ref IOError | ||
| when isMainModule: | ||
| import waku/common/utils/nat, waku/waku_api/message_cache | ||
| import waku/common/utils/nat, waku/waku_rest/message_cache |
There was a problem hiding this comment.
Another possible option is naming it as "rest_api", to keep the consistency with othes like "kernel_api"
| import waku/common/utils/nat, waku/waku_rest/message_cache | |
| import waku/common/utils/nat, waku/rest_api/message_cache |
There was a problem hiding this comment.
I can buy in this idea! I just wanted to name as less things possible as API.
But yes, rest interface is an api anyway.
I make the change!
Description
In relation to #3432 we identified name clashes in nwaku implementation.
As now we are implementing waku-api as the only public interface for waku, we would like to "hide" the complex raw interface of the node.
Also we refered REST API interface as waku_api, that can cause misunderstanding.
In order to clean this up, with this PR I would like to suggest a more clarify namings throughout the code base.
Changes
This pr is not changing any functionality, every change is the result of renaming of directories and imports (and some adjustments to make everything compile).
waku/waku_api/restrenamed towaku/waku_rest/endpointwaku/node/apirenamed towaku/node/kernel_api- I foundkernelas describing best that the underlying interfaces are internal and likely to be changed without notice.waku/apiremains the only top level public API for waku in the future.waku/api.nimwaku-api will be top level import for using waku as a nimble module.Issue
#3432