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

cli: implement db create/drop for sqlite3 #351

Merged
merged 3 commits into from Nov 8, 2017

Conversation

nathanj
Copy link
Contributor

@nathanj nathanj commented Nov 2, 2017

When running amber db create for an sqlite3 database, amber displays
the error:

could not determine database name

This can be confusing to the user who doesn't know that amber db create does not need to be run for sqlite3 databases. Display a
friendlier message.

Also implement amber db drop to delete the database.

@faustinoaq
Copy link
Contributor

Yeah, I can reproduce this, A workaround is using amber migrate up

Copy link
Contributor

@eliasjpr eliasjpr left a comment

Choose a reason for hiding this comment

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

There is no test written for this.

@faustinoaq
Copy link
Contributor

@nathanj Yeah, please add some test for amber db drop and amber db create(sqlite) 😅

@nathanj
Copy link
Contributor Author

nathanj commented Nov 3, 2017

I added a test to init_spec.cr since I didn't know where else to put it. Let me know if there is a better place

@eliasjpr
Copy link
Contributor

eliasjpr commented Nov 3, 2017

@nathanj you can add the specs in spec/amber/cli/commands/generator_spec.cr. Please keep the tests clean and readable follow the pattern

When running `amber db create` for an sqlite3 database, amber displays
the error:

  could not determine database name

This can be confusing to the user who doesn't know that `amber db
create` does not need to be run for sqlite3 databases. Display a
friendlier message.

Also implement `amber db drop` to delete the database.
@nathanj
Copy link
Contributor Author

nathanj commented Nov 4, 2017

Moved the tests and made them a little more spec-like I think (this is my first time using spec).

@eliasjpr
Copy link
Contributor

eliasjpr commented Nov 8, 2017

@nathanj Thank you we were just talking about your PR 🎉

@eliasjpr eliasjpr merged commit 8d9a461 into amberframework:master Nov 8, 2017
elorest pushed a commit that referenced this pull request Nov 17, 2017
When running `amber db create` for an sqlite3 database, amber displays
the error:

  could not determine database name

This can be confusing to the user who doesn't know that `amber db
create` does not need to be run for sqlite3 databases. Display a
friendlier message.

Also implement `amber db drop` to delete the database.
elorest pushed a commit that referenced this pull request Nov 17, 2017
When running `amber db create` for an sqlite3 database, amber displays
the error:

  could not determine database name

This can be confusing to the user who doesn't know that `amber db
create` does not need to be run for sqlite3 databases. Display a
friendlier message.

Also implement `amber db drop` to delete the database.

Former-commit-id: 3253ec9
@faustinoaq faustinoaq added this to Done in Framework 2018 May 5, 2018
@faustinoaq faustinoaq removed this from Done in Framework 2018 Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants