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

Closes #805 - updated db/schema.rb to Rails 5.2 dump #806

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

lentschi
Copy link
Contributor

@lentschi lentschi commented Feb 5, 2021

The format of db/schema.rb has changed in Rails 5 and we should upgrade ours to keep the git history clean for future migrations.

Note: I noticed that db:migrate produced lines like these:
create_table "article_categories", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t|

That is, of course, a problem with non-mysql-like DBs (should be fixed in Rails 6.1) , so I simple manually stripped the options part and it now seems to work fine in mysql-like DBs and sqlite.

@paroga paroga merged commit 64113ab into foodcoops:master Feb 5, 2021
@paroga
Copy link
Member

paroga commented Feb 5, 2021

BTW: i personally do not see a big benefit in creating an issue in addition to a PR. i personally see it as a possible problem since it, has the potential to split a discussion about a change into multiple parts.

@lentschi
Copy link
Contributor Author

lentschi commented Feb 5, 2021

BTW: i personally do not see a big benefit in creating an issue in addition to a PR. i personally see it as a possible problem since it, has the potential to split a discussion about a change into multiple parts.

I'm not sure, what you're getting at there 🤔 Discussions on the issue should be about what to change and how.
Discussions in the PRs should be only about technicalities such as keeping the git history clean.

@paroga
Copy link
Member

paroga commented Feb 5, 2021

Discussions in the PRs should be only about technicalities such as keeping the git history clean.

agree, but just opening an issue parallel to the PR adds IMHO no value. anyway I don't mind if you do, just wanted to let you know that you don't need to follow that organizational rule for such simple non-controversial PRs

@lentschi lentschi deleted the issue_805 branch February 7, 2021 08:53
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