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

Fixed reading the QR code as text in the AuthenticationCommand on dev environments #205

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

mharte-ib
Copy link
Contributor

The Authentication command wasn't able to read the QR code as text because it would try to convert to a string twice.
Also added PHPStan for all dev code and improved some other reported problems, improving code quality.

@MKodde
Copy link
Member

MKodde commented Sep 10, 2024

Thanks for these improvements and the actual bugfix!

You certainly earned your boyscout badge today 🥇

Some small suggestions to make the commit messages more consistent with the ones currently in the git-log I personally try to follow the so called 'golden rules' for a good commit message:

https://dev.to/mitchiemt11/crafting-commits-with-finesse-the-7-rules-for-exceptional-git-commit-messages-2d8b You could set a commit message template to help you stick to those rules. I believe our (Ibuilidngs) 'lessons learned' Gitlab repository contains a template for this.

Edit: did not find the template in our Gitlab; but have a look at this article instead. This is what most of use use for the commit.template

https://codeinthehole.com/tips/a-useful-template-for-commit-messages/

Prior to this change none of the code in the dev folder was being scanned by PHPStan, resulting in a lot of errors which do not comply with our coding standards.

This change is needed to improve code quality for our dev code. All existing errors are added to the baseline.
Prior to this change the type was said to be a string, but we couldn't be sure.
This change solves the issue by typing the result as mixed, then validating whether it was a string.
With the introduction of PHPStan scanning the dev folder a lot of errors were added to the baseline.

I've taken a short time to look at the newly introduced errors and fixed some of them.
@mharte-ib mharte-ib force-pushed the bugfix/dev-authentication-command branch from 1c78f60 to 10422b2 Compare September 11, 2024 05:58
@mharte-ib
Copy link
Contributor Author

Thanks for these improvements and the actual bugfix!

You certainly earned your boyscout badge today 🥇

Some small suggestions to make the commit messages more consistent with the ones currently in the git-log I personally try to follow the so called 'golden rules' for a good commit message:

https://dev.to/mitchiemt11/crafting-commits-with-finesse-the-7-rules-for-exceptional-git-commit-messages-2d8b You could set a commit message template to help you stick to those rules. I believe our (Ibuilidngs) 'lessons learned' Gitlab repository contains a template for this.

Edit: did not find the template in our Gitlab; but have a look at this article instead. This is what most of use use for the commit.template

https://codeinthehole.com/tips/a-useful-template-for-commit-messages/

I've updated the commit messages, making them more explanatory.

@MKodde MKodde merged commit 384732b into main Sep 11, 2024
2 checks passed
@MKodde MKodde deleted the bugfix/dev-authentication-command branch September 11, 2024 12:29
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