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

SQLite open database recipe #464

Merged
merged 1 commit into from
Oct 5, 2018
Merged

SQLite open database recipe #464

merged 1 commit into from
Oct 5, 2018

Conversation

lnicola
Copy link
Contributor

@lnicola lnicola commented Oct 2, 2018 •
edited
Loading

fixes #453, #454

Based on #460. I'll rebase afterwards, please ignore the first two commits.

CC @robertDurst as per #455.

Things to check before submitting a PR

  • the tests are passing locally with cargo test
  • cookbook renders correctly in mdbook serve -o
  • commits are squashed into one and rebased to latest master
  • PR contains correct "fixes #ISSUE_ID" clause to autoclose the issue on PR merge
  • spell check runs without errors ./ci/spellchecker.sh
  • link check runs without errors link-checker ./book
  • non rendered items are in sorted order (links, reference, identifiers, Cargo.toml)
  • links to docs.rs have wildcard version https://docs.rs/tar/*/tar/struct.Entry.html
  • example has standard error handling
  • code identifiers in description are in hyperlinked backticks

Things to do after submitting PR

  • check if CI is happy with your PR

@lnicola lnicola changed the title Sqlite open Sqlite open database recipe Oct 2, 2018
@lnicola
Copy link
Contributor Author

lnicola commented Oct 2, 2018

Should I drop error-chain from this example, like in #433?

@lnicola lnicola force-pushed the sqlite-open branch 2 times, most recently from d49fc77 to d3a5000 Compare October 3, 2018 05:19
@lnicola
Copy link
Contributor Author

lnicola commented Oct 3, 2018

Right. Now what?

  • use bundled everywhere
  • use bundled on Windows
  • figure out how to include SQLite in the AppVeyor build
  • ignore the test

The third option is probably the best, but I don't know anything about the AppVeyor configuration.

@lnicola lnicola mentioned this pull request Oct 3, 2018
Open
@lnicola
Copy link
Contributor Author

lnicola commented Oct 3, 2018

Possibly related to budziq/rust-skeptic#87.

Can we just ignore these tests?

@lnicola
Copy link
Contributor Author

lnicola commented Oct 3, 2018

Fixed, no_run does the trick.

AndyGauge
AndyGauge requested changes Oct 3, 2018
View reviewed changes
src/database/sqlite/initialization.md Outdated

[![rusqlite-badge]][rusqlite] [![cat-database-badge]][cat-database]

You can use the `rusqlite` crate to open SQLite databases. Note that, if you
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personal pronouns should not be used in the description. Refer to the code not the author or the audience. In this case, just drop 'You can'

src/database/sqlite/initialization.md Outdated
[![rusqlite-badge]][rusqlite] [![cat-database-badge]][cat-database]

You can use the `rusqlite` crate to open SQLite databases. Note that, if you
are compiling on Windows, you'll have to perform some [additional steps][linking]
Copy link
Collaborator

Choose a reason for hiding this comment

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

See the [bundled] feature for compiling in Windows.

@lnicola
Copy link
Contributor Author

lnicola commented Oct 4, 2018

Done.

@lnicola lnicola changed the title Sqlite open database recipe SQLite open database recipe Oct 4, 2018
lnicola added a commit to lnicola/rust-cookbook that referenced this pull request Oct 4, 2018
@lnicola
Add SQLite database open recipe (rust-lang-nursery#464)
fd63ab8
AndyGauge
AndyGauge reviewed Oct 5, 2018
View reviewed changes
src/database/sqlite/initialization.md Outdated

[`Connection::open`]: https://docs.rs/rusqlite/*/rusqlite/struct.Connection.html#method.open

[linking]: https://github.com/jgallagher/rusqlite#user-content-notes-on-building-rusqlite-and-libsqlite3-sys
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to match the link above (documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

AndyGauge
AndyGauge reviewed Oct 5, 2018
View reviewed changes
src/links.md Outdated
@@ -114,7 +120,7 @@ Keep lines sorted.
[serde-json-badge]: https://badge-cache.kominick.com/crates/v/serde_json.svg?label=serde_json
[serde-json]: https://docs.rs/serde_json/*/serde_json/
[serde]: https://docs.rs/serde/
[std-badge]: https://badge-cache.kominick.com/badge/std-1.25.0-blue.svg
[std-badge]: https://badge-cache.kominick.com/badge/std-1.29.1-blue.svg
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

AndyGauge
AndyGauge reviewed Oct 5, 2018
View reviewed changes
CONTRIBUTING.md Outdated
@@ -20,7 +20,7 @@ cd rust-cookbook
Cookbook is built with [mdBook], so install that first with Cargo:
Copy link
Collaborator

Choose a reason for hiding this comment

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

git checkout origin/master -- CONTRIBUTING.md

AndyGauge
AndyGauge reviewed Oct 5, 2018
View reviewed changes
src/science.md Outdated
@@ -5,9 +5,18 @@
| [Creating complex numbers][ex-creating-complex-numbers] | [![num-badge]][num] | [![cat-science-badge]][cat-science] |
| [Adding complex numbers][ex-adding-complex-numbers] | [![num-badge]][num] | [![cat-science-badge]][cat-science] |
| [Mathematical functions on complex numbers][ex-mathematical-functions] | [![num-badge]][num] | [![cat-science-badge]][cat-science] |
| [Calculating the side length of a triangle][ex-calculating-side-length-of-triangle] | [![std-badge]][std] | [![cat-science-badge]][cat-science] |
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you git checkout origin/master -- src/science.md and the other science files and push this? I need clean commit history

AndyGauge
AndyGauge requested changes Oct 5, 2018
View reviewed changes
Copy link
Collaborator

@AndyGauge AndyGauge left a comment

Choose a reason for hiding this comment

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

The Database work looks good, but there are files that aren't part of the database section included.

@lnicola
Copy link
Contributor Author

lnicola commented Oct 5, 2018 •
edited
Loading

I rebased again. I don't know why those commits showed up -- I don't think they were there before. Can you check?

@AndyGauge AndyGauge merged commit 78a99d0 into rust-lang-nursery:master Oct 5, 2018
@lnicola lnicola mentioned this pull request Oct 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyGauge AndyGauge AndyGauge 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.

Recipe: Creating Database in File System
2 participants
@lnicola @AndyGauge