freeCodeCamp / chapter

A self-hosted event management tool for nonprofits
BSD 3-Clause "New" or "Revised" License
1.92k stars 360 forks source link

feat: add instance settings to schema #2425

Closed Sboonny closed 1 year ago

Sboonny commented 1 year ago

Closes #2408

ghost commented 1 year ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/freeCodeCamp/chapter/2425/1e203e7a/81ed4446ed439ddb377957f9e1e100a5daa0d2f5.svg)](https://app.codesee.io/r/reviews?pr=2425&src=https%3A%2F%2Fgithub.com%2FfreeCodeCamp%2Fchapter) #### Legend CodeSee Map legend
ojeytonwilliams commented 1 year ago

Sorry, @Sboonny, I didn't see @gikf's comment (and your conversation) when I made my suggestion.

While there are some things we can do to ensure that the table has one record, I would just go with @gikf's approach, but with a comment on the migration to explain why we're not generating ids automatically.

ojeytonwilliams commented 1 year ago

Sorry for the delay on this one, but I think it's a bad idea to update the database without including the code that makes use of those updates. Reason being I can't follow the logic about how this is used. Also, more generally each PR should "do one thing", but this PR "does half a thing". I hope that makes sense!

Could you combine this PR with https://github.com/freeCodeCamp/chapter/pull/2467 ?