Skip to content
This repository has been archived by the owner on May 3, 2021. It is now read-only.

Settings pk #221

Merged
merged 4 commits into from
Mar 28, 2019
Merged

Settings pk #221

merged 4 commits into from
Mar 28, 2019

Conversation

rnwgnr
Copy link
Contributor

@rnwgnr rnwgnr commented Feb 27, 2019

Not a functional addition, but rather a change for consistency:
Add a primary key for the settings table as well.

This will reduce potential misbehavior as setting keys can no longer be set more than once.
This update will fail on installations where the settings are already messed up.

@ildyria ildyria self-requested a review February 28, 2019 02:40
@ildyria ildyria added this to the v3.2.14 milestone Feb 28, 2019
Copy link
Member

@ildyria ildyria left a comment

Choose a reason for hiding this comment

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

Looks good to me.

However this PR will probably wait for other items to be added into Lychee v3 as it switching version just to add a primary key into a table seems a bit small.

@rnwgnr
Copy link
Contributor Author

rnwgnr commented Feb 28, 2019

However this PR will probably wait for other items to be added into Lychee v3 as it switching version just to add a primary key into a table seems a bit small.

Yeah, of course. I may have some rather specific additional features in the next days, even though i`m not sure if these are interesting for "the masses". ;)

@rnwgnr
Copy link
Contributor Author

rnwgnr commented Mar 1, 2019

The setting key 'full_photo' is already implemented but not present in the settings table.
Add this key to the settings table to allow editing using the 'full settings' page instead of messing around with a db tool.

@d7415 d7415 merged commit 08c6227 into LycheeOrg:master Mar 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants