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

Implemented Issue #13 - Add reader mode information to reader object #15

Closed
wants to merge 2 commits into from

Conversation

beamerblvd
Copy link

Re-do of pull request #14. Implemented issue #13. Also rebased to master so that it's merge-able. Compiled libmaxminddb locally, compiled maxminddb.extension against it locally, and tested with all modes explicit and with MODE_AUTO. Works great!

My only concern now is that, in pull request #14, I could access the mode with this:

reader.metadata().mode

But now I have to do this:

reader._db_reader.mode

That's violating best practices by accessing a "private" variable. Suggestions?

@beamerblvd
Copy link
Author

So I don't really understand the output of the failed builds. If I need to fix something, let me know. I do know that it compiles and works on my machine, but I also know that isn't enough.

@oschwald
Copy link
Member

Although we are open to a PR along these lines, this appears to be causing segmentation faults under Travis. Also, new additions such as this should have unit tests.

@oschwald
Copy link
Member

Closing due to inactivity. It would be nice to expose the metadata though so please feel free to reopen once the segmentation faults are resolved.

@oschwald oschwald closed this Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants