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 update metadata #1064

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Add update metadata #1064

merged 2 commits into from
Jul 31, 2023

Conversation

Jarema
Copy link
Member

@Jarema Jarema commented Jul 31, 2023

Signed-off-by: Tomasz Pietrek tomasz@nats.io

@Jarema Jarema force-pushed the add-update-metadata branch 4 times, most recently from 28126bb to 529b467 Compare July 31, 2023 10:03
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
@Jarema Jarema force-pushed the add-update-metadata branch 2 times, most recently from c33d831 to 748f3fd Compare July 31, 2023 10:12
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
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.

One nit, other than that LGTM!

) -> Result<ObjectInfo, UpdateMetadataError> {
let mut info = self.info(object.as_ref()).await?;

// If name is being update, we need to check if other metadata with it already exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// If name is being update, we need to check if other metadata with it already exists.
// If name is being updated, we need to check if other metadata with it already exists.

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 932aa8b into main Jul 31, 2023
13 checks passed
@Jarema Jarema deleted the add-update-metadata branch July 31, 2023 16:42
@Jarema Jarema mentioned this pull request Sep 21, 2023
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.

None yet

3 participants