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

Add kitgen README #743

Merged
merged 3 commits into from
Aug 30, 2018
Merged

Add kitgen README #743

merged 3 commits into from
Aug 30, 2018

Conversation

ermik
Copy link
Contributor

@ermik ermik commented Aug 3, 2018

This measly PR with absolutely no code change and no performance improvements seeks to address #639 with a bit of note-taking.

/request @nyarly

ermik added 3 commits August 3, 2018 10:22
- add installation and running instructions
- add disclaimer
- add introdctory note on experimental nature of the project
- github markdown apparently fails to keep track of `1. - 1. - 1.` style list if block code is involved
- thought it was the double space at the end of the code block, but removing didn't help
@ermik
Copy link
Contributor Author

ermik commented Aug 3, 2018

👇🏻haven't touched those failing tests.

PostProfile(ctx context.Context, p Profile) error
// ...
}
type Profile struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nyarly I went off your profilesvc example in writing this, and now I realize that kitgen doesn't work on an input containing type definitions (and single interface). Not sure what's up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nyarly — found what broke the codegen. See #745

@peterbourgon
Copy link
Member

This seems reasonable to me but I'll wait for a plus-one from @nyarly

@ermik
Copy link
Contributor Author

ermik commented Aug 28, 2018

@peterbourgon perhaps add him as a reviewer?

@nyarly
Copy link
Contributor

nyarly commented Aug 28, 2018

LGTM. Sorry about the delay here.

I didn't remember that there was a dependency on inlinefiles. Should look at that and see if it can be migrated to the more conventional go:generate pattern.

@ermik
Copy link
Contributor Author

ermik commented Aug 29, 2018

@nyarly if you want a tester — I am running kitgen almost daily atm.

@peterbourgon peterbourgon merged commit a57ebce into go-kit:master Aug 30, 2018
@peterbourgon
Copy link
Member

Thanks!

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.

3 participants