Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add name to object metadata #304

Merged
merged 3 commits into from
Jun 1, 2022
Merged

feat: Add name to object metadata #304

merged 3 commits into from
Jun 1, 2022

Conversation

xprazak2
Copy link
Contributor

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Adds a method to metadata which returns name of the object. Any hints on where it should be used to make clippy happy?

@Xuanwo
Copy link
Member

Xuanwo commented May 28, 2022

Thanks for the contribution!

Based on our current design, I expect our name() to be:

  • End with / if it's a folder, otherwise a file (keep the same behavior with path and id)
  • Metadata::name() -> &str and Object::name() -> String (keep the same style with path)

I'm not sure if Path::new() is a good choice for us. Maybe split by / is good enough? (We will consider windows support after #137 with a clear RFC)

I'm open to any ideas, though.

@Xuanwo Xuanwo changed the title feat: Add name to object metadata (#150) feat: Add name to object metadata May 28, 2022
@Xuanwo Xuanwo linked an issue May 28, 2022 that may be closed by this pull request
4 tasks
@xprazak2
Copy link
Contributor Author

xprazak2 commented Jun 1, 2022

Thank you for a review and hints. The trailing slash turns out to be surprisingly tricky as std::path::Path seems to be normalizing them away. Also, when there is a trailing / and I split by it, I cannot add it back and return &str due to ownership, so I return String. Is that ok?

@Xuanwo
Copy link
Member

Xuanwo commented Jun 1, 2022

Is that ok?

LGTM, let's go merging this PR! Thanks for your contribution.

@Xuanwo
Copy link
Member

Xuanwo commented Jun 1, 2022

Run cargo fmt to make our check happy 😄

Signed-off-by: Xuanwo <github@xuanwo.io>
@Xuanwo Xuanwo merged commit 93580c4 into apache:main Jun 1, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stat op support more fields
2 participants