-
Notifications
You must be signed in to change notification settings - Fork 4
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
MDTT-38: Fix for CI failure. #24
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
============================================
- Coverage 20.47% 18.40% -2.07%
- Complexity 163 168 +5
============================================
Files 20 20
Lines 503 603 +100
============================================
+ Hits 103 111 +8
- Misses 400 492 +92 ☔ View full report in Codecov by Sentry. |
@hussainweb cc @shwetaneelsharma As this project was originated as a part of coe, kindly review this PR when you have some time. |
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.
CodeCov is throwing a lot of errors. If the code coverage is not important, consider disabling the check temporarily.
@hussainweb Thank you for your feedback and for reviewing the pull request. I'd like to add a note that I didn't intentionally add the CodeCov integration or the code coverage check; it was preexisting. I'm uncertain about its importance, especially since I've noticed that PRs have been merged despite code coverage issues. Considering your suggestion, I agree that we should temporarily disable the code coverage check. This will prevent the check failures and allow us to merge the PR promptly. We can then discuss the necessity of code coverage and decide whether to re-enable the check in the future. |
@sadeesh-sdet, if the changes here are meeting your needs for the moment, let's merge this and then work on all the comments in separate PRs. What do you say? |
…t an array of strings
…d and add custom exception class
…d and add custom exception class
@hussainweb I think we've already addressed everything except code coverage enablement and its suitable exploration, which we can work on in the future based on priority! |
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.
Sorry, it took so long to review. Please see my comments and I think these should be simple changes.
@hussainweb Likewise, I was tied up with urgent client tasks. I'll attend to the feedback shortly. |
…ed built-in InvalidArgumentException class
@hussainweb I have addressed the PR feedbacks. Please review and merge the changes. |
Description
This pull request addresses the following changes:
PHPStan Improvement: Used a combination of type hinting and explicit checks to assist PHPStan in understanding the types involved and resolving static analysis errors in the
issetField
function.Dependency Security Check: Added a security check for installed dependencies using the
symfonycorp/security-checker-action@v4
action.The rationale behind these changes is to enhance code quality by addressing PHPStan static analysis issues and to ensure the security of the project by checking for known vulnerabilities in dependencies.
Ticket: MDTT-38
Type of Change
Scope (component, file, endpoint, etc.):
issetField
function in [file path]