Skip to content

v2.0.0 #611

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

Closed
wants to merge 53 commits into from
Closed

v2.0.0 #611

wants to merge 53 commits into from

Conversation

gjrtimmer
Copy link
Collaborator

@gjrtimmer gjrtimmer commented Jul 20, 2018

Update driver to v2.0.0

  • Complete Rewrite
  • Move all documentation to Wiki
  • Extendend documentation
  • Repaired examples
  • Provided README for all examples including build commands
  • Implemented Golang Project layout
  • Implemented Pre-Update Hook from Add support for pre-update hooks #597
  • Written Additional Tests
  • Increased coverage
  • Implemented missing functionality for Go:1.10.x
    • *Config
    • driver.Connector
    • driver.DriverContext
  • Rewrite of DNS Options
  • Rewrite of CryptEncoder
    • User can now easily create own CryptEncoder and register is with the driver so it can be used from the DSN.
  • Added support for PPC Architecture
  • Rewrite of the BackupTests because o Go:1.8
  • Add tests for several features
  • Fix race error
  • Added GitHub Templates

This will close PR #597 and Issue #598

@gjrtimmer gjrtimmer requested a review from mattn July 20, 2018 14:35
@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented Jul 20, 2018

This requires manual merge because the master has to be overwritten with this new branch.

@mattn If you want I can do the merge after your approval. It requires a forced push to the master

@gjrtimmer
Copy link
Collaborator Author

PR #439 and PR #487 will be done in the new v2.0.0; I will do those by hand and close the corresponding PR.

@mattn
Copy link
Owner

mattn commented Jul 20, 2018

I prefer to use error message leading with lower case letters. For example: failed to ... not Failed to .... If I had wrote upper case, please fix in this PR.

@gjrtimmer
Copy link
Collaborator Author

I understand not a problem. I will fix this and add it to the contributing guide. Anything else you want or prefer? I honestly don't mind because you started this great repo. Any rules or requirements you want shall be added to the contributing guide which I have added in this PR.

@mattn
Copy link
Owner

mattn commented Jul 23, 2018

As far as I can see, patckage path is moved to driver directory, So this break compatibility?

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented Jul 23, 2018

Yes, and second what breaks backwards compatibility are the names of the DSN. No longer is prefix '_' required for the options within the DSN, because there are no conflicts between the options of SQLite and this package.

Check out the Wiki.

Closes #450

[skip ci]
@gjrtimmer
Copy link
Collaborator Author

@mattn Also several options for the DSN have been renamed. _auth_user for example is now simple user. On the Wiki page of the DSN I have even created a conversion list between pre-2.0.0 versions and the 2.0.0 version.

See Conversion List

@gjrtimmer
Copy link
Collaborator Author

Also _loc is now fully implemented as tz (Timezone). Within the DSN as well within the Config.

@mattn
Copy link
Owner

mattn commented Jul 23, 2018

I'm feeling that the impact of this breaking backward compatibility for third party packages is too big. Most of app or packages using go-sqlite3 is not managed by package manager like glide, dep or vgo. I wonder this proposal requires somewhat unreasonable work for users. May be, we will get many vote down comments. :-(

So, how about to separate repository? Move to github.com/go-sqlite3/v1 or v2, Or use pkg.in ?

BTW, your works is great. Thanks.

@gjrtimmer
Copy link
Collaborator Author

@mattn Thanks. We could do a separate repository however I'm not a fan for it.
I do not think that we will receive a lot of votes down.

We have significantly increased the documentation. Provided conversion tables and we follow Semantic Versioning. We because things are not entirely backwards compatible we increased the Major version to 2.

Users can also always checkout tag 1.9.0; I honestly think users will be happy with a major update. I think we should update it and if we really get much backfire; we can either look into a rollback or providing the community with additional conversion documentation.

@gjrtimmer
Copy link
Collaborator Author

BTW, Native full encryption (SEE), build upon the v.2.0.0 is almost done

@gjrtimmer
Copy link
Collaborator Author

@mattn see previous post; If we really get to many complaints we always can create the separate repository like to suggest after the complaints; I really think we should update it and just see.

Adding a large text to the beginning to the README as a warning about the backwards compatibility should help to.

@gjrtimmer
Copy link
Collaborator Author

@mattn Any thought yet.

P.S. I've completed SEE native Golang encryption by incorperating any other repo and makes quite some change to get it all working. But it is working perfectly. Also I have several other updates ready.

@mattn
Copy link
Owner

mattn commented Jul 24, 2018

Could you please explain all of differences between v1 and v2 ? I want to know how many breaking backword compatibilities. Thanks.

@gjrtimmer
Copy link
Collaborator Author

@mattn

  • _ Prefix removed from DSN configuration keys
  • Renamed DSN config key loc to tz and added timezone as alternative
  • Dropped config key _auth
  • Removed prefix _auth_ from UserAuthentication config keys
  • Implemented Golang project structure; moved the driver to folder driver, this changes the import path. package name sqlite3 remains the same. Import path is now github.com/mattn/go-sqlite3/driver
Pre-2.0.0 2.x
cache cache
mutex mutex
_auth N.A.
_auth_user user
_auth_pass pass
_auth_salt salt
_auth_crypt crypt
_auto_vacuum auto_vacuum
_busy_timeout busy_timeout
_case_sensitive_like case_sensitive_like
_cslike cslike
_defer_foreign_keys defer_foreign_keys
_defer_fk defer_fk
_foreign_keys foreign_keys
_fk fk
_ignore_check_constraints ignore_check_constraints
_journal_mode journal_mode
_journal journal
_loc tz, timezone
_locking_mode locking_mode
_locking locking
_query_only query_only
_recursive_triggers recursive_triggers
_rt rt
_secure_delete secure_delete
_synchronous synchronous
_sync sync
_timeout timeout
_txlock txlock
_vacuum vacuum
_writable_schema writable_schema

Additional documentation moved from README to Wiki Pages.

@mattn
Copy link
Owner

mattn commented Jul 24, 2018

_ Prefix removed from DSN configuration keys

I'm thinking this might occur problem when sqlite include new flag which have same name in go-sqlite3.

Implemented Golang project structure; moved the driver to folder driver, this changes the import path. package name sqlite3 remains the same. Import path is now github.com/mattn/go-sqlite3/driver

Sorry, I still not understand well why this is needed.

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented Jul 24, 2018

_ Prefix removed from DSN configuration keys

I'm thinking this might occur problem when sqlite include new flag which have same name in go-sqlite3.

The chance that a conflict will arise will be small because the keys that are given to SQLite are for low-level configuration. Currently there is no such change on the roadmap. Second if their might be a conflict in the future we can easily change it. I think however that the chance will be extremely slim that we will have a conflict in the future.

Implemented Golang project structure; moved the driver to folder driver, this changes the import path. package name sqlite3 remains the same. Import path is now github.com/mattn/go-sqlite3/driver

Sorry, I still not understand well why this is needed.

This was done by my choice, I you want to revert it; it will be fine. To explain my decision. This layout will give us the opportunity to add additional folders and applications, libraries to the project.

What I was thinking about is to add a Web UI service / Shell to the project in the future. In which you could manage your SQLite database. Available as shell program and library so you can embed it within you project. If you look at the README of Golang Project Layout then you will see where I got the idea.

I've done it with my eye on the future to expand en develop this repository even further. If you do not want this. It will be fine with me.

I was just looking toward the future, and version v2.0.0 gave me a chance to make some big changes. That's all there is to it.

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented Jul 24, 2018

@mattn See previous post for explanation.

@mattn If you want to see the end result. I've updated the master of my own fork to v2.0.0 including the latest fixes for the features from fix/features as well as the new SEE full database encryption.

You can find the documentation for it within my Wiki Pages. I hope you like it. Take a look

@gjrtimmer
Copy link
Collaborator Author

@mattn within my own repo I've reversed the SEE; this requires some additional work.

@mattn
Copy link
Owner

mattn commented Jul 27, 2018

What I was thinking about is to add a Web UI service / Shell to the project in the future. In which you could manage your SQLite database. Available as shell program and library so you can embed it within you project. If you look at the README of Golang Project Layout then you will see where I got the idea.

I prefer to add cmd/sqlite-shell or something. I am concerned about changing the import path.

Since I am conservative, I always think that I would like to prevent existing users from asking for changes.

Please give me more time to think about this.

@gjrtimmer
Copy link
Collaborator Author

@mattn Not a problem take all the time you need. Please check out other SQL drivers like; Postgres, MySQL maybe those will help you choose what to do.

Journal Mode should only be enforced when the user set it.

Fixes: #607
strip old SEE Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants