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.

Sign up for GitHub

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

Jump to bottom

issues7 | createAddXml in CompatibilityLayer #8

Closed
wants to merge 3 commits into from
Closed

issues7 | createAddXml in CompatibilityLayer #8

wants to merge 3 commits into from

Conversation

GZyl
Copy link

@GZyl GZyl commented Apr 17, 2014

issue 7
Add methods removed attributes

GZyl added 3 commits April 17, 2014 18:38
@GZyl
issues7 | createAddXml in CompatibilityLayer
5eb443d
@GZyl
issues7 |
cac23fd 
- addDocuments - fixed
@GZyl
issues7 |
b71ba41 
- testAddDocumentWithSolr4
@GZyl GZyl mentioned this pull request Apr 17, 2014
Closed
@rjkip rjkip mentioned this pull request Apr 24, 2014
Merged
@rjkip
Copy link
Contributor

rjkip commented Apr 24, 2014

Hi @GZyl,

Thanks a lot for your contribution. You did a good job. I have opted not to break backwards compatibility. Based on your implementation and tests, I have implemented a solution in #9. Could you review this and confirm that this solves #7?

Thanks!

@GZyl
Copy link
Author

GZyl commented Apr 24, 2014

;] #9 solves #7 but it is a little more complicated (We can use it).

There was backwards compatibility
We made test on solr 3.6 and solr 4.7.

@rjkip
Copy link
Contributor

rjkip commented Apr 24, 2014

Backwards compatibility was broken in case someone implemented their own comp layer. Not likely, but a BC break nonetheless. So you agree on merging #9?

@GZyl
Copy link
Author

GZyl commented Apr 24, 2014

I agree.

@rjkip rjkip closed this Apr 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GZyl @rjkip