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

add SystemErrno to Error #740

Merged
merged 3 commits into from Dec 17, 2019
Merged

add SystemErrno to Error #740

merged 3 commits into from Dec 17, 2019

Conversation

rittneje
Copy link
Collaborator

fixes #678

@rittneje
Copy link
Collaborator Author

@mattn @gjrtimmer sqlite3_system_errno was added in SQLite 3.12.0. I guess whatever version you are using in the libsqlite3 tests is older than that. I don't see anywhere in your README that mentions a minimum required version of libsqlite3. How would you like me to proceed?

@coveralls
Copy link

coveralls commented Aug 24, 2019

Coverage Status

Coverage increased (+0.2%) to 50.622% when pulling 4581f04 on rittneje:feature/678-add-system-errno-to-error-struct into d3c6909 on mattn:master.

@rittneje
Copy link
Collaborator Author

@mattn @gjrtimmer I was able to leverage the C pre-processor to work around the version issue.

@gjrtimmer
Copy link
Collaborator

@mattn @gjrtimmer sqlite3_system_errno was added in SQLite 3.12.0. I guess whatever version you are using in the libsqlite3 tests is older than that. I don't see anywhere in your README that mentions a minimum required version of libsqlite3. How would you like me to proceed?

The libsqlite3 test is using the library on the Travis CI system. Don't know if we can update to force a specific version.

@gjrtimmer
Copy link
Collaborator

@rittneje On first glance, this PR look great.

@mattn
Copy link
Owner

mattn commented Aug 27, 2019

Sounds good to me.

@gjrtimmer
Copy link
Collaborator

@mattn What is the default we are using for merging "Create a Merge Request" of "Squash and merge"

@mattn
Copy link
Owner

mattn commented Aug 28, 2019

I don't have strong opinion. Feel free to create rule if you want. :)

@rittneje
Copy link
Collaborator Author

@gjrtimmer @mattn Can this be merged?

@rittneje
Copy link
Collaborator Author

ping @mattn

@mattn mattn merged commit b4f5cc7 into mattn:master Dec 17, 2019
@mattn
Copy link
Owner

mattn commented Dec 17, 2019

Thanks

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.

Add low-level system error code to Error struct
4 participants