Improve exception handling and code consistency#128
Improve exception handling and code consistency#128SamMousa merged 1 commit intoCodeception:masterfrom
Conversation
SamMousa
left a comment
SamMousa
left a comment
There was a problem hiding this comment.
Reviewed from mobile, might not catch all.
Lgtm. I'd love a code sniffer rule for the import of global classes Vs the \xxx notation.
src/Codeception/Module/Yii2.php
Outdated
| * Adds the required server params. | ||
| * Note this is done separately from the request cycle since someone might call | ||
| * `Url::to` before doing a request, which would instantiate the request component with incorrect server params. | ||
| * @return array<string, mixed> |
There was a problem hiding this comment.
Isn't every value a string? We can be even more specific and use array shape.
There was a problem hiding this comment.
It should be, but it could be misconfigured.
Since it is possible to write any type of value in $this->config['entryScript'] and we are not validating the type of data received anywhere I get what you see in the screenshot.
I changed it to use array shape, I hope that's what you meant.
the port was returned manually as string, but from parse_url it is returned as int, let me know if you prefer an explicit cast to string (string) or if you are satisfied with string|int.
TavoNiievez
force-pushed
the
small-fixes
branch
from
98d570d to
c43be6a
Compare
|
Will review soon, reminder: use conventional commit messages. https://www.conventionalcommits.org/en/v1.0.0/ Otherwise automated releases won't work. For now I can fix it in the merge commit message. |
TavoNiievez
force-pushed
the
small-fixes
branch
4 times, most recently
from
5e0c782 to
f9da3fe
Compare
|
Overall cosmetic changes with a few changes for behavior that I see as good ones 👍 |
- Standardized `RuntimeException` usage by removing redundant namespace references.
- Refactored `findAndLoginUser` for clarity using ternary assignment.
- Used `readonly` property for `ignoreCollidingDSN` in `TransactionForcer`.
- Optimized server params handling by introducing `getServerParams()`.
- Improved configuration validation messages for clean and mail methods.
|
LGTM |
|
🎉 This PR is included in version 2.0.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
No description provided.