Skip to content
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

WIP: Remove buf.yaml parsing from CreateCommits #17

Closed
wants to merge 1 commit into from

Conversation

bufdev
Copy link
Member

@bufdev bufdev commented Nov 6, 2023

Naming, exact layout, etc still WIP, but for our discussion.

@bufdev bufdev closed this Nov 6, 2023
bufdev added a commit that referenced this pull request Nov 6, 2023
…d module and dependency content (#20)

This replaces #17 as #16 was closed.

This removes the sending of `buf.yaml` and `buf.lock` files over the
wire, and consequently these being part of the structure of `Modules` as
we know them, instead sending over the specific information we need on
the backend in a structured manner.

This fully standardizes on the `Node` terminology. A `Node` is a pointer
to some content, either on the request or response side. Within this API
now, we have:

- `FileNode`: A pointer to file content.
- `CreateCommitRequest.ModuleNode` - A set of `FileNodes` and an
associated `ModuleRef` that represents the content of a single
`Module.`.
- `CreateCommitRequest.DepNode` - A set of pointers to dependencies,
which are just the `commit_ids` of the dependencies and their associated
`Digest` (for verification).
- `CreateCommitResponse.CommitNode` - A set of pointers to files and
then actual commit dependencies.

This renames `GetFileNodes` to `GetCommitNodes`.

To be honest, I don't love the `Node` naming, and I think it could use
some work, but that's less important for the purpose of this PR. We can
work on that. Ideas I've had:

- `Info` for new nested types, ie `ModuleInfo, DepInfo, CommitInfo`.
- `Data` for new nested types. In general, I think it's an anti-pattern
to name a Protobuf message `.*Data`, as all Protobuf messages are data,
and it doesn't pluralize well.

Open to other suggestions.
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.

1 participant