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

Update README.md with use for SuppliedSchema #17

Merged
merged 4 commits into from
Feb 10, 2020
Merged

Conversation

j-halbert
Copy link

  • Example in README.md for use of post_schema updated to show that
    SuppliedSchema struct in example is also member of schema_registry
    module.

Related to issue #16

- Example in `README.md` for use of `post_schema` updated to show that
`SuppliedSchema` struct in example is also member of `schema_registry`
module.
- Use declaration removes `SubjectNameStrategy`, as `post_schema`
doesn't belong to this enum.
- `post_schema` get the created `SuppliedSchema` as argument, which is
required.
@gklijs
Copy link
Owner

gklijs commented Feb 10, 2020

Currently the build i failing, but somehow it doesn't show here. I'm almost done resolving the problems in a local branch, and think adding url crate is a nice addition. Just have to make sure any error in the url is properly handled. Will go back to it tonight.

@gklijs gklijs merged commit 4e0c185 into gklijs:master Feb 10, 2020
@j-halbert
Copy link
Author

Thanks for merging this. I was a little sloppy with this, as I didn't intend to bring the url crate in on this pull request.

We're using it in our project to ensure correctly formatted urls, but found that it appended a / to all addresses, which wasn't compatible with how schema registry converter was using urls.

Anyway, I had planned to make a separate pull request for this with all the proper updates to tests so you didn't have to do that. Sorry this got included with the wrong pull request.

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.

None yet

3 participants