Skip to content

implement TextMarshaler/JSONMarshaler more consistently #2097

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

Merged
merged 2 commits into from
May 17, 2025

Conversation

imjasonh
Copy link
Collaborator

Various things in the Go ecosystem natively understand how to go from string->Foo if Foo implements TextUnmarshaler, and likewise from Foo->string if Foo implements TextMarshaler.

This adds TextMarshaler and JSONMarshaler support to name.Registry, name.Repository, name.Tag, and name.Digest, and makes v1.Hash's implementation a bit more consistent with those (using json.Unmarshal instead of strconv.Unquote)

@imjasonh imjasonh requested review from jonjohnsonjr and Copilot May 16, 2025 20:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds consistent JSON and text (un)marshaling support to v1.Hash and the name package types (Tag, Repository, Registry, Digest).

  • Switches v1.Hash from strconv.Unquote to json.Unmarshal and condenses marshal methods.
  • Implements MarshalJSON, UnmarshalJSON, MarshalText, and UnmarshalText for Tag, Repository, Registry, and Digest.
  • Removes the unused strconv import in pkg/v1/hash.go.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
pkg/v1/hash.go Updates Hash JSON methods and removes strconv import
pkg/name/tag.go Adds JSON/Text marshaling for Tag
pkg/name/repository.go Adds JSON/Text marshaling for Repository
pkg/name/registry.go Adds JSON/Text marshaling for Registry
pkg/name/digest.go Adds Text marshaling for Digest
Comments suppressed due to low confidence (2)

pkg/v1/hash.go:21

  • The code calls json.Marshal and json.Unmarshal but the encoding/json package is not imported; add "encoding/json" to the imports.
import (

pkg/name/tag.go:112

  • [nitpick] Consider adding unit tests for Tag.MarshalJSON and Tag.UnmarshalJSON to ensure correct serialization and error handling.
func (t Tag) MarshalJSON() ([]byte, error) { return json.Marshal(t.String()) }

Signed-off-by: Jason Hall <[email protected]>
@imjasonh imjasonh merged commit a61de15 into google:main May 17, 2025
16 checks passed
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.

2 participants