Skip to content

Gabriel/Step one#43

Open
grrlopes wants to merge 1 commit intobutton-inc:mainfrom
grrlopes:step_one
Open

Gabriel/Step one#43
grrlopes wants to merge 1 commit intobutton-inc:mainfrom
grrlopes:step_one

Conversation

@grrlopes
Copy link

Step one was completely done according to instructions on readme .
Do not forget to change sqitch.conf db addr to localhost or whenever your
db is running.

@grrlopes grrlopes changed the title Step one structure. Gabriel/Step one Jan 17, 2023
Copy link
Contributor

@dleard dleard left a comment

Choose a reason for hiding this comment

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

This looks great! I just added a small comment on naming conventions when deciding what to name your database tables.

-- requires: todoAppschema

BEGIN;
CREATE TABLE todo_app.todo_create_table (
Copy link
Contributor

Choose a reason for hiding this comment

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

a small nitpick on table naming convention:
"todo_create_table" is kind of a confusing name for this table. A table name should describe the data that it contains.
The "thing" that this table contains is a "todo", so I would just name this table "todo"

Copy link
Author

Choose a reason for hiding this comment

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

a small nitpick on table naming convention: "todo_create_table" is kind of a confusing name for this table. A table name should describe the data that it contains. The "thing" that this table contains is a "todo", so I would just name this table "todo"

it makes sense. deal !!

By the way, I had thought something like:
todo = what is this table for.
behavior = create , alter or remove.
table = schema table.

Copy link
Author

Choose a reason for hiding this comment

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

Your way makes more sense due to sqitch
has desc on plan through flag -n

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.

2 participants