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

Fixes #3840 - Removes unused Signo related code #1740

Closed
wants to merge 1 commit into from

Conversation

ares
Copy link
Member

@ares ares commented Sep 5, 2014

I'm on doubt whether we should also add a migration that would remove signo_sso and signo_url settings from database. Ideas?

@lzap
Copy link
Member

lzap commented Sep 6, 2014

I think we can add manual upgrade step for this one.

Do we have a page or branch with 1.7 upgrade instructions already? Let's
just put it there... ;-)

LGTM

Later,
Lukas #lzap Zapletal

@ohadlevy
Copy link
Member

ohadlevy commented Sep 6, 2014

why not remove the setting? it would linger forever on old installs?

@ares
Copy link
Member Author

ares commented Sep 8, 2014

I always try to avoid data manipulation in migrations since it can break easily. Deletion is probably the safest operation so I'll add it using some FakeSetting class.

@ares
Copy link
Member Author

ares commented Sep 8, 2014

Migration added

@GregSutcliffe
Copy link
Member

Seems fine:

sqlite> select * from settings where name like '%sign%';
67|signo_sso||Use Signo SSO for login|Setting::Auth|boolean|--- false
68|signo_url||Signo SSO url|Setting::Auth|string|--- http://url/

$ bundle exec rake db:migrate
==  RemoveSignoSetting: migrating ==
==  RemoveSignoSetting: migrated (0.0012s) ==

sqlite> select count(*) from settings where name like '%sign%';
0

Merged as 4fdfa62, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants