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

Document the _proto directory #4756

Merged

Conversation

timthelion
Copy link

I'm only guessing on the use of the rudder service definition. I tried googling to no avail and neither https://github.com/timthelion/helm/blob/master/_proto/hapi/rudder/rudder.proto nor https://github.com/helm/helm/blob/master/cmd/rudder/rudder.go explain it.

@timthelion timthelion force-pushed the timthelion-document-_proto-dir branch from b6a9b9b to 0c5914b Compare October 10, 2018 11:11
Signed-off-by: Timothy Hobbs <timothy@hobbs.cz>
@timthelion timthelion force-pushed the timthelion-document-_proto-dir branch from 0c5914b to 45f388f Compare October 10, 2018 11:21
@timthelion
Copy link
Author

I wonder why the contents of verison.proto are not in tiller.proto. It is confusing to have it in the top level dir when a chart versions are encoded differently (as strings https://github.com/helm/helm/blob/master/_proto/hapi/chart/metadata.proto#L51) .

I also don't understand why the rudder directory is not in the services directory when it is in the services package.

@timthelion
Copy link
Author

Can someone with the correct credentials hit the rebuild button? I don't think that should be failing.

@bacongobbler
Copy link
Member

Just as a note, we do make a note of this in the developer's guide, though I can see why a README may be useful.

@timthelion
Copy link
Author

timthelion commented Oct 10, 2018

The README would not be nearly as useful if the directory name did not start with an underscore and was instead something descriptive like protobuf ;). At first glance, I disregarded the directory as being some auto-generated set of files which had been committed either by mistake or due to the difficulty of installing the utility that so generated them.

@timthelion
Copy link
Author

There is only one other directory which has an unclear name, rootfs and that too has a README file explaining its purpose.

@bacongobbler bacongobbler merged commit ace30b8 into helm:master Nov 6, 2018
@bacongobbler bacongobbler added this to the 2.12.0 - Features milestone Nov 6, 2018
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
Signed-off-by: Timothy Hobbs <timothy@hobbs.cz>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
Signed-off-by: Timothy Hobbs <timothy@hobbs.cz>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
Signed-off-by: Timothy Hobbs <timothy@hobbs.cz>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
Signed-off-by: Timothy Hobbs <timothy@hobbs.cz>
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