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

Changelog for 1.7.2 #786

Closed

Conversation

greg-1-anderson
Copy link

Preliminary.

CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@greg-1-anderson
Copy link
Author

greg-1-anderson commented Sep 5, 2019

Updated per review comments.

One request, if I may. Similar to the situation in minkphp/MinkSelenium2Driver#313, Drupal is also depending on the 1.7.x-dev branch alias for this repository. Could you please create an unmaintained 1.7 branch before changing the branch alias to 1.8.x-dev? Here also we would like the commits that already exist as part of the 1.7.x-dev branch alias that destined for the 1.8.x branch to be placed on the 1.7 branch.

Sorry for the inconvenience.

@aik099
Copy link
Member

aik099 commented Sep 5, 2019

@greg-1-anderson , not all items were addressed. I've resolved conversations for items that were fixed. Others needs to be addressed.

@greg-1-anderson
Copy link
Author

@aik099 : I addressed most of the items that remained unresolved. The exceptions:

Objects with __toString() can be asserted

and having a new features section also means this cannot be a patch release.

I already moved this line up to "features" and bumped the expected version to 1.8.0, but this comment remains unresolved. Was there something else you wanted to happen here?

Modify response[Contains|NotContains] to use strpos vs preg_match

should be removed IMO. This is a change of an internal implementation, without any user-facing impact

Already removed, but this not marked resolved. A mistake, perhaps?

Please let me know if you want any further changes with these items or anything else.

@aik099
Copy link
Member

aik099 commented Sep 6, 2019

Objects with __toString() can be asserted

Resolved.

Modify response[Contains|NotContains] to use strpos vs preg_match

Could you bring it back (the fixed version). I consider this important change.

Now we're waiting for #787, where @stof suggests to make a minor release of every mink repo with current features used for Drupal and then do a feature release, where available.

@stof
Copy link
Member

stof commented Sep 6, 2019

Now we're waiting for #787, where @stof suggests to make a minor release of every mink repo with current features used for Drupal and then do a feature release, where available.

I don't understand what you mean. minor versions are precisely the versions receiving features in semver.

@stof
Copy link
Member

stof commented Sep 6, 2019

Modify response[Contains|NotContains] to use strpos vs preg_match

Could you bring it back (the fixed version). I consider this important change.

Why ? that's an internal performance optimization to avoid using a regex when not necessary. The behavior is exactly the same.

@aik099
Copy link
Member

aik099 commented Sep 6, 2019

#786 (comment)

@stof , my bad. I thought, that:

  • BC breaks goes into major release
  • features goes into feature release
  • bugs go into minor release

But instead I should be thinking:

  • BC breaks goes into major release
  • features goes into major release
  • bugs go into patch release

I understand now.

#786 (comment)

@stof , if people use these methods as expected by us, then yes, nothing is changed. However they could have passed-in regex fragments before and that worked as well. That won't work anymore now.

@stof
Copy link
Member

stof commented Sep 6, 2019

if people use these methods as expected by us, then yes, nothing is changed. However they could have passed-in regex fragments before and that worked as well. That won't work anymore now.

this was not possible before either. We were properly escaping their input with preg_quote.

@greg-1-anderson
Copy link
Author

Based on the above, it sounds to me as if the preg_match removal was not a significant change. I'm ready to bring that comment back if you decide you want it, though.

@aik099
Copy link
Member

aik099 commented Sep 6, 2019

#786 (comment)

@stof , fine. Then it's good, that @greg-1-anderson removed it.

@aik099
Copy link
Member

aik099 commented Sep 6, 2019

@greg-1-anderson , since all what I've requested was done I'm approving the PR.

@scaytrase
Copy link

scaytrase commented Sep 16, 2019

Any news here? It's still a problem to use mink with sf4 without hacks

@neclimdul
Copy link

Is this still happening? Would be really great to have a new release.

@aik099
Copy link
Member

aik099 commented Dec 12, 2019

I guess we're waiting for @stof review and then we can merge it. Not sure if anything else is blocking the release though.

@alexpott
Copy link
Contributor

alexpott commented Mar 3, 2020

It would be really really great to have PHP 7.4 compatibility in this release. The smallest fix for that is #792 but there's also #794

@aik099
Copy link
Member

aik099 commented Mar 5, 2020

It would be really really great to have PHP 7.4 compatibility in this release. The smallest fix for that is #792 but there's also #794

@alexpott , I've approved #792 (@stof can merge it unless he has) and commented upon #794.

@stof
Copy link
Member

stof commented Mar 11, 2020

I ended up doing my own update of the changelog to make the release. Some of the bug fixes listed in this changelog are actually fixes compared to the previous PR merged, not fixes compared to the previous release, and so there is no point highlighting them in the changelog (the entry would read we avoided regressions when refactoring if comparing to the previous release).

@stof stof closed this Mar 11, 2020
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.

7 participants