Skip to content

Remove unused authentication_token column#236

Merged
tvdeyen merged 1 commit into
solidusio:mainfrom
mamhoff:remove-unused-columns
Jan 8, 2025
Merged

Remove unused authentication_token column#236
tvdeyen merged 1 commit into
solidusio:mainfrom
mamhoff:remove-unused-columns

Conversation

@mamhoff

@mamhoff mamhoff commented Dec 2, 2022

Copy link
Copy Markdown
Contributor

Summary

This column dates back to 2013, when token authentication was removed from Devise. We should not keep it.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

  • I have written a thorough PR description.
  • I have kept my commits small and atomic.
  • I have used clear, explanatory commit messages.

The following are not always needed (cross them out if they are not):

  • I have added automated tests to cover my changes.
  • I have attached screenshots to demo visual changes.
  • I have opened a PR to update the guides.
  • I have updated the README to account for my changes.

@jarednorman jarednorman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My only concern here is for stores that decided to use this column for something... I would not want to drop all their "authentication tokens", whatever they put in there.

Comment thread db/migrate/20221202120901_remove_unused_authentication_token.rb Outdated
This column dates back to 2013, when token authentication was removed
from Devise. We should not keep it.

This allows new stores to start without the unused column, while old
stores can still use any tokens they might be using.
@mamhoff mamhoff force-pushed the remove-unused-columns branch from 156fabf to 2b2e0de Compare January 6, 2025 14:28
@tvdeyen tvdeyen merged commit f535321 into solidusio:main Jan 8, 2025
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.

3 participants