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

Making Uri proto, microuri support mandatory and longuri support optional #110

Merged

Conversation

ruchirchauhan
Copy link
Contributor

@ruchirchauhan ruchirchauhan commented Apr 9, 2024

This PR implements issue #102

It modifies uri.proto:

  1. UEntity name is now optional
  2. UResource name is now optional

In order to make microuri support standard and mandatory,
all the "Id" fields need to be required fields and the corresponding
"name" fields have to be optional.
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.

The purpose of this change is to allow us to build URIs that contain only numbers that could be used for micro and short URI format however if we put id as mandatory and name optional, we will not be able to build URIs that are in long format only (meaning URIs that only contain names). Since UUri is a container to store the URI information, it needs to support the ability to contain in its belly:

  1. Names only
  2. Numbers only
  3. Names & numbers

The changes I've proposed allows the UUri to contain any of the above information, please consider the suggestions

up-core-api/uprotocol/uri.proto Outdated Show resolved Hide resolved
up-core-api/uprotocol/uri.proto Outdated Show resolved Hide resolved
Make all the fields optional, to allow long form only and micro form only.
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.

Approved

@stevenhartley stevenhartley merged commit 3cefb30 into eclipse-uprotocol:main Apr 9, 2024
2 checks passed
@sophokles73
Copy link
Contributor

@stevenhartley it would be nice if we could squash PRs like this before merging in the future to keep the commit log in a clean and consistent state. We now have three commits in the log, each of which represents intermediate work and which only make sense when applied together ...

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