-
Notifications
You must be signed in to change notification settings - Fork 0
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
[STORY-648] feat: simplify passwords generation #17
[STORY-648] feat: simplify passwords generation #17
Conversation
* default password length is raised from 20 to 24 characters long * generated passwords contain only alphanumeric and underscore characters * not related to the issue, upgrade to go 1.23
CHANGELOG.md
Outdated
@@ -1,6 +1,10 @@ | |||
# Changelog | |||
|
|||
## To be Released | |||
## 1.0.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version bump is not supposed to happen in this PR. It will be part of the release of a new version (https://github.com/Scalingo/gopassword?tab=readme-ov-file#release-a-new-version).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oups, revert is coming...
go.mod
Outdated
@@ -1,6 +1,6 @@ | |||
module github.com/Scalingo/gopassword | |||
|
|||
go 1.20 | |||
go 1.23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: Go 1.23 is not installed on the servers. Only Go 1.22 (https://github.com/Scalingo/appsdeck-kitchen/blob/7b6e1f5f12eba1d3f24f6da13214cf1dc5874254/environments/production.rb#L15-L18). Hence I would only update this parameter to 1.22
As per our servers Go version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: the title of the PR does not match our current practice. The PR title should follow the Karma convention (https://karma-runner.github.io/6.4/dev/git-commit-msg.html). Hence I would have expected something like: feat: simplify password generation
.
CHANGELOG.md
Outdated
@@ -2,6 +2,10 @@ | |||
|
|||
## To be Released | |||
|
|||
* chore(go): use go 1.23 | |||
* Raise default length from 20 to 24 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this new default length (24) the minimum required by our PSSI?
edit: after reading the related story, I think the default length should be the maximum length. Hence it should be 64. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The minimum (24) is enough but the maximum (64) is better.
Go for 64 !!
Co-authored-by: Étienne M. <EtienneM@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 🎉
characters
STORY-648