feat: add branch/tag metadata maps and tag timestamps#6364
feat: add branch/tag metadata maps and tag timestamps#6364majin1102 wants to merge 3 commits intolance-format:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca167a2b8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ca167a2 to
cb7b97b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb7b97bda1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
4bec46e to
bcbeed3
Compare
bcbeed3 to
a080281
Compare
westonpace
left a comment
There was a problem hiding this comment.
The change seems clean and straightforward. What is your motivation for branch / tag metadata? Arguably this might be a spec change (requiring 3 votes) though it isn't actually modifying any .proto files.
| | `createdAt` | string | Yes | RFC 3339 timestamp for when the tag was first created. | | ||
| | `updatedAt` | string | Yes | RFC 3339 timestamp for the latest tag reference update. | |
There was a problem hiding this comment.
Since we have manifest_size should this be created_at and updated_at for consistency?
Or actually, looking at the test, I wonder if this is supposed to be manifestSize below?
| for (tag_name, tag_content) in tags { | ||
| let dict = PyDict::new(py); | ||
| dict.set_item("version", tag_content.version)?; | ||
| dict.set_item("created_at", tag_content.created_at)?; |
There was a problem hiding this comment.
Do you also need to set branch?
| """ | ||
| Replace metadata for an existing tag. | ||
| """ |
There was a problem hiding this comment.
Can you expand on this a little? Will this merge in the metadata or completely replace any existing metadata? Will this cause updated_at to change?
Thanks for driving this forward. For reference, the motivation thread is here: And the vote thread is here: |
8999446 to
7ab286b
Compare
What Changed
metadatato structured key/value maps.createdAt/created_atupdatedAt/updated_atRust
TagContentsnow includes:metadata: HashMap<String, String>created_at: Option<DateTime<Utc>>updated_at: Option<DateTime<Utc>>created_atand refreshupdated_at.Python
branchversioncreated_atupdated_atmanifest_sizemetadataTagTypedDict.Tags.list()docstring to describe the full returned metadata.Java
Tagnow includes:Map<String, String> metadataOptional<Instant> createdAtOptional<Instant> updatedAtDocs