Skip to content

[feature/2fa] Simplify 2fa features #121

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

Open
wants to merge 3 commits into
base: feature/2fa
Choose a base branch
from

Conversation

laserhybiz
Copy link
Contributor

@laserhybiz laserhybiz commented Apr 23, 2025

This PR to feature/2fa branch contains 3 commits for the following:

  • Moved migration into user migration (unlike jetstream a new project is not built on top of the base skeleton)
  • Use model casts for encryption/decryption
  • Run pint

@laserhybiz laserhybiz changed the title Simplify 2fa features [feature/2fa] Simplify 2fa features Apr 23, 2025
@tnylea
Copy link
Contributor

tnylea commented Apr 23, 2025

Thanks, @laserhybiz. Why do you want to move the migrations into the users' migration? My reasoning for keeping it the same is that people will likely want to upgrade, and including a separate file (the 2fa columns migration) will make it easier for users to move files, rather than editing current files.

Let me know your thoughts. Thanks!

@laserhybiz
Copy link
Contributor Author

@tnylea
I think it depends on how people typically handle updates. In my experience, most applications already have modifications to the original skeleton files. Personally, when I want to update to the latest changes, I prefer editing the current files (commit-by-commit) rather than copying entire files over. In that case, I’d just create a new migration for my project if needed. I think keeping the skeleton clean by updating existing migrations is better than adding separate ones for each change.

@tnylea tnylea added the On Hold This PR may be on hold for any particular reason label Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Hold This PR may be on hold for any particular reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants