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

remove dead code related to activating users and setting passwords #170

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

briskt
Copy link
Contributor

@briskt briskt commented Sep 6, 2024

Removed

  • Removed dead code related to activating users and setting passwords

Feature PR Checklist

  • Unit tests created or updated

@briskt briskt requested a review from a team September 6, 2024 05:53
Copy link

sonarqubecloud bot commented Sep 6, 2024

Copy link

@hobbitronics hobbitronics left a comment

Choose a reason for hiding this comment

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

Looks good. I'm not familiar with the code so wait for more reviews if you like

@briskt
Copy link
Contributor Author

briskt commented Sep 9, 2024

Looks good. I'm not familiar with the code so wait for more reviews if you like

Thanks. I doubt this is going to be familiar to anyone. It was just something I stumbled across when studying the APIs that are called between IdP services.

@briskt briskt merged commit 2f3696d into develop Sep 9, 2024
3 checks passed
@briskt briskt deleted the feature/remove-dead-code branch September 9, 2024 08:05
Copy link
Contributor

@forevermatt forevermatt left a comment

Choose a reason for hiding this comment

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

Nice cleanup. I had to think for a bit to be comfortable that these aren't used anymore, but I believe you're correct. 👍

Just had one little suggested tweak/addition, if it's worth it.

*/
public function aUserExists()
public function anActiveUserExists()
Copy link
Contributor

Choose a reason for hiding this comment

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

It might not be a bad idea to also have this assert that the user is active, since that is what it's name now conveys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants