Skip to content
This repository has been archived by the owner on May 15, 2023. It is now read-only.

Added protocol buffer models #5

Merged
merged 7 commits into from
Jul 30, 2020
Merged

Conversation

margaritiko
Copy link
Contributor

@margaritiko margaritiko commented Jul 29, 2020

Add protocol buffer models for representing bindings, dependencies and graph

All models constructed with proto2. This is preferred because in proto3 all fields are optional which can add ambiguity.

  • Binding is represented by its unique string-key.

  • Dependency model represents an edge in graph which always contains target node.

  • BindingGraph contains a map adjacency_list which simply matches binding or component (access it by string representation of key) to list with other bindings.

@margaritiko margaritiko added the enhancement New feature or request label Jul 29, 2020
@margaritiko margaritiko added this to the Implement queries milestone Jul 29, 2020
@margaritiko margaritiko requested a review from Ratchette July 29, 2020 17:07
@margaritiko margaritiko requested a review from allie-wood as a code owner July 29, 2020 17:07
@margaritiko margaritiko self-assigned this Jul 29, 2020
@margaritiko margaritiko force-pushed the protocol_buffers_models branch from 744d42a to bce4d6e Compare July 29, 2020 17:11
@@ -0,0 +1,1932 @@
// Generated by the protocol buffer compiler. DO NOT EDIT!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All these files with Proto suffix are generated automatically, so don't waste your time on them ⏰

Choose a reason for hiding this comment

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

How about excluding generated code from source control, by marking them in .gitignore? And maybe add a command for the code generation in the README. This way it will reduce clutter and prevent the generated / actual source code from ever going out of sync :)

Copy link
Contributor Author

@margaritiko margaritiko Jul 30, 2020

Choose a reason for hiding this comment

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

Cool, thanks! I think it's good idea, I will add entire autogen folder into .gitignore

@margaritiko margaritiko force-pushed the protocol_buffers_models branch from bce4d6e to f4e825c Compare July 29, 2020 17:14
@kwchan1995
Copy link

Perhaps controversial, actually the internal guidance is to prefer optional protobuf fields to required, because it makes future refactorings easier without having to support the required fields indefinitely (see https://developers.google.com/protocol-buffers/docs/proto#specifying_field_rules and protocolbuffers/protobuf#2497 (comment)). This is also why required was removed in proto3. Would it be possible to move to proto3 for future-proofing if not too much work please?

@margaritiko margaritiko force-pushed the protocol_buffers_models branch from 6682344 to 9660862 Compare July 30, 2020 13:55
@@ -4,6 +4,17 @@ A graph based query language on top of [dagger](https://github.com/google/dagger

Dagger Query can be used to investigate the dagger dependency graph.

## Before you start
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding this documentation to the readme is a really good idea.

@margaritiko margaritiko merged commit e71c0e6 into master Jul 30, 2020
@margaritiko margaritiko deleted the protocol_buffers_models branch August 17, 2020 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants