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.

Sign up for GitHub

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

Jump to bottom

[PM-3435] Passphrase generator #262

Merged
merged 18 commits into from
Dec 4, 2023
Merged

[PM-3435] Passphrase generator #262

merged 18 commits into from
Dec 4, 2023

Conversation

dani-garcia
Copy link
Member

Type of change

- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Added passphrase generator, including bw cli support

@dani-garcia dani-garcia requested a review from Hinton September 29, 2023 18:33
@bitwarden-bot
Copy link

bitwarden-bot commented Sep 29, 2023 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – 2b05e8ed-f717-4869-9528-f085151ec88e

No New Or Fixed Issues Found

@dani-garcia dani-garcia force-pushed the ps/pm-3435-passphrase-generator branch from 2386905 to c056e91 Compare September 29, 2023 18:54
Hinton
Hinton requested changes Oct 6, 2023
View reviewed changes
Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

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

This review uses Conventional commits.

I think this is an excellent start, the password.rs file is getting a bit large and we can benefit from separating the two generators. We can break up the generation in a few functions that are easily unit tested which will increase our confidence in the code long term.

crates/bitwarden/src/tool/generators/password.rs Show resolved Hide resolved
crates/bitwarden/src/tool/generators/password.rs Outdated Show resolved Hide resolved
crates/bitwarden/src/tool/generators/password.rs Outdated Show resolved Hide resolved
@dani-garcia dani-garcia requested a review from Hinton October 9, 2023 11:36
@dani-garcia dani-garcia requested a review from Hinton October 10, 2023 11:03
Hinton
Hinton previously approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

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

Looks great, thanks.

Hinton
Hinton previously approved these changes Oct 10, 2023
View reviewed changes
audreyality
audreyality requested changes Oct 11, 2023
View reviewed changes
Copy link
Member

@audreyality audreyality left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • ❌, ⚠️ - BLOCKS APPROVAL - for more significant problems or concerns needing attention
  • 💭 - for open-ended inquiry that's not quite a confirmed issue and could potentially benefit from discussion

crates/bitwarden/src/tool/generators/passphrase.rs Outdated Show resolved Hide resolved
crates/bitwarden/src/tool/generators/passphrase.rs Outdated Show resolved Hide resolved
crates/bitwarden/src/tool/generators/passphrase.rs Show resolved Hide resolved
@Hinton Hinton requested a review from audreyality October 13, 2023 09:00
audreyality
audreyality previously approved these changes Oct 25, 2023
View reviewed changes
Copy link
Member

@audreyality audreyality left a comment

Choose a reason for hiding this comment

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

Just do what you're going to do.

@fedemkr fedemkr mentioned this pull request Nov 7, 2023
Merged
@dani-garcia
Merge branch 'master' into ps/pm-3435-passphrase-generator
97d83e5 
# Conflicts:
#	crates/bitwarden/src/tool/generators/mod.rs
@dani-garcia dani-garcia requested a review from Hinton December 4, 2023 12:25
@dani-garcia dani-garcia merged commit 7fc3b84 into master Dec 4, 2023
40 checks passed
@dani-garcia dani-garcia deleted the ps/pm-3435-passphrase-generator branch December 4, 2023 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hinton Hinton Hinton approved these changes

@audreyality audreyality Awaiting requested review from audreyality

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dani-garcia @bitwarden-bot @Hinton @audreyality