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

Add Zenoh spec to uProtocol #52

Merged
merged 15 commits into from
Feb 29, 2024

Conversation

evshary
Copy link
Contributor

@evshary evshary commented Jan 17, 2024

The PR is not ready to be merged, but I think it would be easier to discuss the spec if we open a draft PR.
I'll keep working on uprotocol-rust-ulink-zenoh and updating the spec here.
Feel free to add any comments on this.

Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@stevenhartley
Copy link
Contributor

PR to address #21

@evshary evshary marked this pull request as ready for review February 26, 2024 15:15
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
@PLeVasseur
Copy link
Contributor

Thanks @evshary, I think it's just about there. 🙂 I think adding a link to the appropriate part of the up-spec detailing how Micro Form URIs work and then continuing to describe that particular example would be a good way to tie things together.

Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

Please see comments

up-l1/zenoh.adoc Outdated
=== Zenoh Config

While constructing the UPClientZenoh object, the Zenoh config needs to be assigned.
Copy link
Contributor

Choose a reason for hiding this comment

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

This section will need to be outlined a little bit more. Perhaps for now we can remove it and then add it in later (so we have a starting ground to build on top of?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I think removing this part in the initial version will not affect the interoperability.
Why we need Zenoh config is that we need a way to configure how Zenoh works.
Let's discuss it a bit more in the meeting.

up-l1/zenoh.adoc Show resolved Hide resolved
up-l1/zenoh.adoc Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Feb 28, 2024

@stevenhartley @PLeVasseur Thanks for your review. Maybe we can discuss the rest reviews in the weekly meeting.

@@ -5,7 +5,7 @@
The key words "*MUST*", "*MUST NOT*", "*REQUIRED*", "*SHALL*", "*SHALL NOT*", "*SHOULD*", "*SHOULD NOT*", "*RECOMMENDED*", "*MAY*", and "*OPTIONAL*" in this document are to be interpreted as described in https://www.rfc-editor.org/info/bcp14[IETF BCP14 (RFC2119 & RFC8174)]

----
Copyright (c) 2023 General Motors GTO LLC
Copyright (c) 2024 ZettaScale Technology
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to use

SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation

as suggested by https://www.eclipse.org/projects/handbook/#ip-copyright-headers and https://reuse.software/tutorial/#step-2 and then remove the redundant entry below the license header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean removing the line SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation and only use Copyright (c) 2024 Contributors to the Eclipse Foundation instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean to replace line

Copyright (c) 2024 ZettaScale Technology

with

SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to use

SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation

as suggested by https://www.eclipse.org/projects/handbook/#ip-copyright-headers and https://reuse.software/tutorial/#step-2 and then remove the redundant entry below the license header.

@sophokles73 I'll add this best practice to the CONTRIBUTING.adoc in .github and then slowly update all the projects to point to this CONTRIBUTING.adoc file.

up-l1/zenoh.adoc Show resolved Hide resolved
up-l1/zenoh.adoc Outdated Show resolved Hide resolved
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
Signed-off-by: ChenYing Kuo <evshary@gmail.com>
@evshary
Copy link
Contributor Author

evshary commented Feb 29, 2024

@stevenhartley @PLeVasseur The spec is updated. Let me now if there is anything missing.

@PLeVasseur
Copy link
Contributor

Yeah, I think if you update the copyright header as suggested by Kai, I think we're good 🙂

up-l1/zenoh.adoc Show resolved Hide resolved
@@ -5,7 +5,7 @@
The key words "*MUST*", "*MUST NOT*", "*REQUIRED*", "*SHALL*", "*SHALL NOT*", "*SHOULD*", "*SHOULD NOT*", "*RECOMMENDED*", "*MAY*", and "*OPTIONAL*" in this document are to be interpreted as described in https://www.rfc-editor.org/info/bcp14[IETF BCP14 (RFC2119 & RFC8174)]

----
Copyright (c) 2023 General Motors GTO LLC
Copyright (c) 2024 ZettaScale Technology
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to use

SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation

as suggested by https://www.eclipse.org/projects/handbook/#ip-copyright-headers and https://reuse.software/tutorial/#step-2 and then remove the redundant entry below the license header.

@sophokles73 I'll add this best practice to the CONTRIBUTING.adoc in .github and then slowly update all the projects to point to this CONTRIBUTING.adoc file.

@stevenhartley stevenhartley merged commit f703310 into eclipse-uprotocol:main Feb 29, 2024
1 check passed
@evshary evshary deleted the zenoh_spec branch March 1, 2024 01:44
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.

4 participants