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

[5.8] Refactoring the unless method with when in BuildsQueries class #28965

Closed
wants to merge 1 commit into from
Closed

[5.8] Refactoring the unless method with when in BuildsQueries class #28965

wants to merge 1 commit into from

Conversation

arcanedev-maroc
Copy link
Contributor

No description provided.

@devcircus
Copy link
Contributor

The tests for "unless" are now failing.

**WARNING :** Don't merge this PR because of the inconsistency of the correct behavior

This is a breaking change because the `value` from the condition is altered by `$this->when(! $value, ...)`.

I think this is inconsistent with [Illuminate\Support\Collection::unless](https://github.com/laravel/framework/blob/5.8/src/Illuminate/Support/Collection.php#L606)

So, what is the correct behavior ?
@arcanedev-maroc
Copy link
Contributor Author

arcanedev-maroc commented Jun 26, 2019

WARNING : Don't merge this PR because of behavioral Inconsistencies.

This is a breaking change because the value from the condition is altered by $this->when(! $value, ...).

I think this is inconsistent with Illuminate\Support\Collection::unless

So, what is the correct behavior ?
Is this considered as a bug in Illuminate/Support/Collection::unless ?

Related: #19738, #19740

@driesvints
Copy link
Member

If this is a breaking change then please send it to master.

@driesvints driesvints closed this Jun 27, 2019
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.

3 participants