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

Fix unindex convert issue #69 #70

Merged
merged 4 commits into from
Apr 16, 2019
Merged

Fix unindex convert issue #69 #70

merged 4 commits into from
Apr 16, 2019

Conversation

andbag
Copy link
Member

@andbag andbag commented Apr 12, 2019 •
edited by icemac
Loading

The code is now consolidated for general usage of _convert to avoid unnecessary doubling of code.

Fixes #69.

@andbag andbag requested review from icemac and d-maurer April 12, 2019 15:00
@andbag andbag mentioned this pull request Apr 12, 2019
Closed
d-maurer
d-maurer requested changes Apr 12, 2019
View reviewed changes
Copy link
Contributor

@d-maurer d-maurer left a comment

Choose a reason for hiding this comment

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

Something strange happened to the change log (lots of apparently new material).

Otherwise good.

@andbag andbag force-pushed the fix_unindex_convert_issue_69 branch from ba4199e to ed3fca7 Compare April 12, 2019 19:57
@andbag andbag requested a review from d-maurer April 12, 2019 20:10
@andbag
Copy link
Member Author

andbag commented Apr 12, 2019

@d-maurer Sorry for my copy & paste fault. The change log is fixed.

icemac
icemac approved these changes Apr 15, 2019
View reviewed changes
Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

LGTM but I did not look into it very deep. I trust the judgment of @d-maurer.

@icemac
Copy link
Member

icemac commented Apr 15, 2019

@andbag You should be able to merge this PR.

@andbag andbag merged commit 9ac0910 into master Apr 16, 2019
@icemac icemac deleted the fix_unindex_convert_issue_69 branch April 17, 2019 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icemac icemac icemac approved these changes

@d-maurer d-maurer d-maurer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UnIndex applies its _convert method on searching but not on indexing
3 participants
@andbag @icemac @d-maurer