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

microservices header missing #701

Closed
geiseri opened this issue Nov 28, 2023 · 9 comments · Fixed by #731
Closed

microservices header missing #701

geiseri opened this issue Nov 28, 2023 · 9 comments · Fixed by #731
Assignees
Labels
defect Suspected defect such as a bug or regression

Comments

@geiseri
Copy link

geiseri commented Nov 28, 2023

Observed behavior

The header micro_args.h is missing from the install files.

Expected behavior

It is added to the install section of src/CMakeLists.txt

Server and client version

release 3.7.0

Host environment

No response

Steps to reproduce

No response

@geiseri geiseri added the defect Suspected defect such as a bug or regression label Nov 28, 2023
@geiseri
Copy link
Author

geiseri commented Nov 28, 2023

Noted by the header contents that it is incomplete maybe this is blocking based on completion so feel free to close.

@kozlovic
Copy link
Member

@geiseri Since v3.7.0, the service framework was released, so I think this header should have been included indeed, even if there are still TODOs. Will create a PR to address that.

@kozlovic
Copy link
Member

@geiseri Actually, having second thoughts. This is used only for the examples, and not sure that it was intended to be library-level APIs. Asking @levb what we should be doing with that header file.

@geiseri
Copy link
Author

geiseri commented Nov 29, 2023

@kozlovic looking at the internals maybe it should be in the examples directory. The "args format" looks like is specific to the example as it wouldn't have enough of a schema for interop.

@levb levb self-assigned this Jan 22, 2024
@levb
Copy link
Collaborator

levb commented Jan 22, 2024

micro-args was kind of a stop-gap TBH, for examples only. I will move it there.

@levb
Copy link
Collaborator

levb commented Jan 22, 2024

Moving micro_args.* to ./examples would be rather involved since the CMakeLists.txt builds an executable for every .c file.

Moving micro_args.h only, and leaving micro_args.c in ./src can be confusing.

0/5 we should just leave the files where they are, and perhaps replace the use of micro_args in the examples with another, OSS serialization package? Initially I didn't want to pull in the dependencies even for the examples, but using JSON or some other common format would probably be preferred?

@geiseri @kozlovic ?

@kozlovic
Copy link
Member

@levb Maybe the "mistake" was to make this example program a bit too configurable ;-). The parameters could have been hard coded, in that the value of the examples was to show how to use the service APIs, not really to be able to run actual fibonnaci or factorial computations :-)

But I would not want to pull a JSON dependency just for that. As you said, we have tried hard to limit the dependencies, so it would be a shame to add one just for the examples. Note that most of the build issues reported by users are around the protobuf (to compile the NATS Streaming client).

@levb
Copy link
Collaborator

levb commented Mar 14, 2024

I agree. For now, I am proposing a PR that moves all this serialization business into a single .h file under examples. Will make Help wanted tickets for more, simpler examples.

@levb levb closed this as completed in #731 Mar 15, 2024
@geiseri
Copy link
Author

geiseri commented Mar 22, 2024

@levb did you make that ticket? I have some ideas on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants