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.

Sign up for GitHub

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

Jump to bottom

Update Service for parity with ADR #965

Merged
merged 3 commits into from
May 23, 2023
Merged

Update Service for parity with ADR #965

merged 3 commits into from
May 23, 2023

Conversation

Jarema
Copy link
Member

@Jarema Jarema commented May 10, 2023

No description provided.

@Jarema Jarema force-pushed the jarema-add-service-metadata branch 3 times, most recently from 08e570c to 1c0f663 Compare May 11, 2023 07:40
@Jarema Jarema requested a review from caspervonb May 11, 2023 07:47
@Jarema Jarema marked this pull request as ready for review May 11, 2023 07:57
@piotrpio piotrpio self-requested a review May 11, 2023 10:40
piotrpio
piotrpio requested changes May 11, 2023
View reviewed changes
Copy link
Contributor

@piotrpio piotrpio left a comment

Choose a reason for hiding this comment

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

It should be possible to add metadata when adding endpoints as well: https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-32.md#schema

async-nats/src/service/mod.rs Show resolved Hide resolved
@Jarema
Copy link
Member Author

Jarema commented May 11, 2023

I found few more missing bits. Will add them all here.

@Jarema Jarema changed the title Add service metadata Update Service for parity with ADR May 12, 2023
@Jarema Jarema mentioned this pull request May 12, 2023
Closed
@Jarema Jarema marked this pull request as draft May 12, 2023 12:36
@Jarema Jarema force-pushed the jarema-add-service-metadata branch 10 times, most recently from d31b66d to 791a396 Compare May 18, 2023 14:06
@Jarema Jarema marked this pull request as ready for review May 18, 2023 14:24
@Jarema Jarema force-pushed the jarema-add-service-metadata branch 4 times, most recently from 3c204d0 to 897e9ae Compare May 21, 2023 17:46
@Jarema Jarema requested a review from piotrpio May 21, 2023 18:10
@Jarema
Add service metadata
181890d 
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
@Jarema
Add missing parity features for service API
b3e06de 
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
@Jarema Jarema force-pushed the jarema-add-service-metadata branch from 897e9ae to b135488 Compare May 21, 2023 19:21
caspervonb
caspervonb requested changes May 22, 2023
View reviewed changes
Copy link
Collaborator

@caspervonb caspervonb left a comment

Choose a reason for hiding this comment

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

Some minor nits for consideration.

async-nats/src/service/mod.rs Outdated Show resolved Hide resolved
async-nats/src/service/mod.rs Show resolved Hide resolved
async-nats/src/service/mod.rs Outdated Show resolved Hide resolved
async-nats/src/service/mod.rs Outdated Show resolved Hide resolved
async-nats/src/service/mod.rs Show resolved Hide resolved
@Jarema
Reorganise service module
95986ad 
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
@Jarema Jarema force-pushed the jarema-add-service-metadata branch from b135488 to 95986ad Compare May 22, 2023 16:53
@Jarema Jarema requested a review from caspervonb May 22, 2023 16:53
@piotrpio piotrpio requested review from piotrpio and removed request for piotrpio May 23, 2023 07:05
caspervonb
caspervonb approved these changes May 23, 2023
View reviewed changes
Copy link
Collaborator

@caspervonb caspervonb left a comment

Choose a reason for hiding this comment

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

lgtm

@Jarema Jarema merged commit fc81ba6 into main May 23, 2023
@Jarema Jarema deleted the jarema-add-service-metadata branch May 23, 2023 12:07
@Jarema Jarema mentioned this pull request Jul 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caspervonb caspervonb caspervonb approved these changes

@piotrpio piotrpio piotrpio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jarema @caspervonb @piotrpio