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

[STORY-642] new api endpoint to handle password reset #1071

Merged

Conversation

sc-zenokerr
Copy link
Contributor

@sc-zenokerr sc-zenokerr commented Jul 4, 2024

  • Add a changelog entry in the section "To Be Released" of CHANGELOG.md

Database user password update uses new API endpoint for generating password.

This is dependent on the merging / release of the equivalent branch in go-scalingo library.

Story 642

…ssword.

This is dependent on the merging / release of the equivalent branch in go-scalingo library.
@sc-zenokerr sc-zenokerr requested a review from EtienneM July 4, 2024 11:15
Copy link
Member

@EtienneM EtienneM left a comment

Choose a reason for hiding this comment

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

Just a small suggestion but LGTM 👍

Did you ask product about the CLI backward compatibility?

Comment on lines 76 to 77
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: I like returning the function as soon as possible. In this case, you could return here and the else is useless. There is less indentation which makes the code easier to read IMO

Suggested change
}
} else {
}
fmt.Printf("User \"%s\" password updated.\n", databaseUser.Name)
return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I actually misread the original flow. Your suggestion is absolutely correct. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you ask product about the CLI backward compatibility?

Yes. My question was answered by benjamin in the comments in the [STORY-642] in notion.

It should be (remain) possible to set a user-supplied password using the CLI.

@sc-zenokerr sc-zenokerr changed the title Fix/story 642/new api endpoint to handle password reset [STORY-642] new api endpoint to handle password reset Jul 5, 2024
Copy link

@sc-zenokerr sc-zenokerr enabled auto-merge July 8, 2024 10:19
@sc-zenokerr sc-zenokerr merged commit 8e056a9 into master Jul 8, 2024
5 checks passed
@sc-zenokerr sc-zenokerr deleted the fix/story-642/new-api-endpoint-to-handle-password-reset branch July 8, 2024 10:21
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.

2 participants