Skip to content

add config.md#32

Merged
gaius-qi merged 1 commit intomodelpack:mainfrom
caozhuozi:config-md
Feb 14, 2025
Merged

add config.md#32
gaius-qi merged 1 commit intomodelpack:mainfrom
caozhuozi:config-md

Conversation

@caozhuozi
Copy link
Contributor

@caozhuozi caozhuozi commented Feb 11, 2025

fix: #27

Some notes for the reviewer:

  1. I haven't imposed any additional restrictions on the DiffID, as, according to the comment, the format doesn't need to be limited to a cryptographic hash. However, the comment might be outdated, and I'm unsure about the current preference.

  2. Do we need a more concrete example instead of a mock one? My concern is that, due to point 1, the format of DiffID has not been determined yet, which makes it difficult to provide a concrete example at this time.

  3. I haven't imposed any additional restrictions on the properties in config according to that the current status is not stable yet.

docs/config.md Outdated
```json
{
"descriptor": {
"createTime": "2025-01-01T00:00:00Z",
Copy link
Contributor

Choose a reason for hiding this comment

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

The createTime has been change to CreateAt in #31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

@caozhuozi
Copy link
Contributor Author

Do we need additional restrictions on certain properties, such as "format", "quantization", "parameterSize", "architecture", or even "name"?

@caozhuozi caozhuozi force-pushed the config-md branch 4 times, most recently from 73dff1a to 8ccf10c Compare February 12, 2025 05:03
@caozhuozi caozhuozi changed the title [WIP] add config.md add config.md Feb 12, 2025
aftersnow
aftersnow previously approved these changes Feb 12, 2025
Copy link
Contributor

@aftersnow aftersnow left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: caozhuozi <543481992@qq.com>
Copy link
Member

@gaius-qi gaius-qi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

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

lgtm

@gaius-qi gaius-qi merged commit 992a940 into modelpack:main Feb 14, 2025
2 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.

need config.md that describes the model config media type

4 participants