Fix mismatched doc comments on exported weight config constants#204
Fix mismatched doc comments on exported weight config constants#204ilopezluna wants to merge 1 commit intomodelpack:mainfrom
Conversation
Signed-off-by: Ignacio López Luna <ignasi.lopez.luna@gmail.com>
ff2cc24 to
be3e085
Compare
There was a problem hiding this comment.
Code Review
This pull request updates the documentation comments for several media type constants to correctly match their identifiers. The review feedback suggests further refining the phrasing of these comments to improve consistency with other definitions in the codebase.
| MediaTypeModelWeightConfigRaw = "application/vnd.cncf.model.weight.config.v1.raw" | ||
|
|
||
| // MediaTypeModelConfig specifies the media type for configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | ||
| // MediaTypeModelWeightConfig specifies the media type for configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. |
There was a problem hiding this comment.
For better consistency with the MediaTypeModelConfig definition (line 25) and the constant name itself, consider using the phrasing 'model weight configuration' instead of 'configuration of the model weights'. This also aligns with the singular 'weight' used in the identifier and media type string.
| // MediaTypeModelWeightConfig specifies the media type for configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | |
| // MediaTypeModelWeightConfig specifies the media type for a model weight configuration, including files like tokenizer.json, config.json, etc. |
| MediaTypeModelWeightConfig = "application/vnd.cncf.model.weight.config.v1.tar" | ||
|
|
||
| // MediaTypeModelConfigGzip specifies the media type for gzipped configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | ||
| // MediaTypeModelWeightConfigGzip specifies the media type for gzipped configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. |
There was a problem hiding this comment.
For consistency with the base constant and other compressed media types in this file, consider using 'gzipped model weight configuration'.
| // MediaTypeModelWeightConfigGzip specifies the media type for gzipped configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | |
| // MediaTypeModelWeightConfigGzip specifies the media type for a gzipped model weight configuration, including files like tokenizer.json, config.json, etc. |
| MediaTypeModelWeightConfigGzip = "application/vnd.cncf.model.weight.config.v1.tar+gzip" | ||
|
|
||
| // MediaTypeModelConfigZstd specifies the media type for zstd compressed configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | ||
| // MediaTypeModelWeightConfigZstd specifies the media type for zstd compressed configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. |
There was a problem hiding this comment.
For consistency with the base constant and other compressed media types in this file, consider using 'zstd compressed model weight configuration'.
| // MediaTypeModelWeightConfigZstd specifies the media type for zstd compressed configuration of the model weights, including files like `tokenizer.json`, `config.json`, etc. | |
| // MediaTypeModelWeightConfigZstd specifies the media type for a zstd compressed model weight configuration, including files like tokenizer.json, config.json, etc. |
Description
Fix Go doc comments for three exported constants in
specs-go/v1/mediatype.gowhere the comment name did not match the actual constant name:MediaTypeModelWeightConfig— comment incorrectly started withMediaTypeModelConfigMediaTypeModelWeightConfigGzip— comment incorrectly started withMediaTypeModelConfigGzipMediaTypeModelWeightConfigZstd— comment incorrectly started withMediaTypeModelConfigZstdAll three comments were missing the
Weightsegment in the identifier name, causing them to violate Go's convention that doc comments on exported identifiers should start with the identifier's name.Related Issue
No existing issue. This was identified by reviewing the doc comments in the
mediatype.gofile.Motivation and Context
Go doc comments on exported identifiers should start with the identifier name (per Go conventions and
go vet/revivebest practices). These mismatched comments could cause confusion for contributors and consumers of the package, especially when relying on generated documentation (e.g.,pkg.go.dev), where the comment text would reference a non-existent identifier.