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

[MINOR] Updated kafkaServer.md #442

Merged
merged 2 commits into from Feb 9, 2017

Conversation

Projects
None yet
3 participants
@himani1
Copy link
Contributor

commented Feb 9, 2017

No description provided.

@ignasi35

This comment has been minimized.

Copy link
Member

commented Feb 9, 2017

hi @himani1, thanks for the contribution. Unfortunately this change is still no addressing a bigger issue in that part of the documentation.
Th Kafka docs were inspired by the Cassandra docs and we found an issue there yesterday which I just patched: https://github.com/lagom/lagom/pull/443/files

Feel free to port the patch to the Kafka pages or close this issue and I'll take over the task.

Cheers,

@himani1

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

@ignasi35 Hi, I am confused. The PR https://github.com/lagom/lagom/pull/443/files doesn't contain the changes that I have made. And also the md files seem to be different.

@ignasi35

This comment has been minimized.

Copy link
Member

commented Feb 9, 2017

Sorry for the confusion, let me elaborate.
There's the same error in Cassandra pages and Kafka pages when detailing how to connect to external services. In both cases the XML snippet is wrong (in the kafka pages it's even syntactically wrong as you corrected).

The PR #443 fixes the documentation error for "Connecting to external Cassandra". What I wanted to point out is that you syntax fix is very welcome but I'll overwrite it in a couple of minutes.

Again, if it hadn't been for your PR I wouldn't have noticed that the Kafka docs are wrong so I can only but thank you for contributing. :simple_smile:

@himani1

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

@ignasi35 All right I can understand what you meant now. Let me update my PR with the fixes that you made to #443 and you can review it then.

@ignasi35 ignasi35 merged commit cca6e9d into lagom:1.2.x Feb 9, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details

@himani1 himani1 deleted the himani1:patch-1 branch Feb 13, 2017

@TimMoore TimMoore referenced this pull request Mar 14, 2017

Merged

Update KafkaServer.md #593

4 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.