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

refactor: Fix phpstan expr.resultUnused #9385

Conversation

neznaika0
Copy link
Contributor

Description
We just ignore the lines, as we expect an exception.
You can set an assignment $s = $entity->ninth, but it seems unnecessary in the test.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

This can go to develop branch.

@paulbalandan paulbalandan added the testing Pull requests that changes tests only label Jan 8, 2025
@neznaika0
Copy link
Contributor Author

I'm afraid of conflicts between branches. If I start with develop, sometimes this has already been changed in 4.6. Can I push in 4.6?

@samsonasik
Copy link
Member

@neznaika0 we are dealing with try to release 4.6 sooner, if it took some conflict, it can stay here for a while, so next develop already 4.6 so you can change target branch to develop.

@paulbalandan
Copy link
Member

What conflicts are you seeing?

@paulbalandan
Copy link
Member

As a rule, all refactoring should target the lowest branch unless the code in question is present only in the next minor version.

@neznaika0
Copy link
Contributor Author

See changes: #9374 (comment)

While I was spending time searching for the cause, the issue was resolved in 4.6

It's not difficult for me to migrate commits. Maybe there is a tip on how to start working with two branches?

@neznaika0 neznaika0 force-pushed the refactor/phpstan-expr-result-unused branch from 09dd7fc to 06e47f0 Compare January 8, 2025 15:42
@neznaika0 neznaika0 changed the base branch from 4.6 to develop January 8, 2025 15:42
@paulbalandan
Copy link
Member

Well, I'm not aware of that. In such case, what I would maybe do is check if the baselined errors are present in both branches which if then I would target develop. I'm not sure though if that's practical on your part.

@paulbalandan paulbalandan merged commit 3eb8a09 into codeigniter4:develop Jan 8, 2025
49 checks passed
@paulbalandan
Copy link
Member

Thanks so far, @neznaika0 , for cleaning up the baseline.

@neznaika0 neznaika0 deleted the refactor/phpstan-expr-result-unused branch January 8, 2025 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Pull requests that changes tests only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants