Navigation Menu

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

Make validator getClientOptions public #13145

Conversation

nkovacs
Copy link
Contributor

@nkovacs nkovacs commented Dec 6, 2016

Q A
Is bugfix? no
New feature? yes
Breaks BC? yes
Tests pass? yes
Fixed issues #11163

This allows implementing custom client-side validation
without extending every validator.

The backwards compatibility break is caused by the change in ImageValidator. ImageValidator::getClientOptions was protected since 2.0.0 (6dd2203#diff-504fd084646834cd27fce3dbdce26bbeR350), so if you extended the class and overrode the method with a protected method, the visibility change will cause a fatal error (because you can't make a public method protected in a subclass).

Fixes #11163

@cebe
Copy link
Member

cebe commented Dec 6, 2016

The backwards compatibility break is caused by the change in ImageValidator.

could you add that note to the UPGRADE guide please?

@cebe cebe added this to the 2.0.11 milestone Dec 6, 2016
This allows implementing custom client-side validation
without extending every validator.

Fixes yiisoft#11163
@nkovacs nkovacs force-pushed the 11163-validator-getclientoptions-public branch from 4d2c016 to 97ca9dc Compare December 6, 2016 16:13
@nkovacs
Copy link
Contributor Author

nkovacs commented Dec 6, 2016

Done.

@samdark samdark modified the milestones: 2.1.0, 2.0.11 Dec 7, 2016
@samdark
Copy link
Member

samdark commented Dec 7, 2016

Since it's a backwards compatibilty break and isn't critical to have in 2.0.x, I'd merge it into 2.1.0 instead. @cebe ?

@SilverFire
Copy link
Member

I insist to merge this PR in 2.0.11 and here are the reasons:

  • We must admit that once we release an API, it will be used by other developers.
  • Method getClientOptions() was introduced on the high-level API in version 2.0.11. Before that only ImageValidator class used the same method name.
  • In case we merge this PR to 2.1 milestone UPGRADE will affect much more code in comparison to ImageValidator BC break.

@nkovacs
Copy link
Contributor Author

nkovacs commented Dec 7, 2016

There's also the option of calling it something else (though even then you're introducing a BC break, since if you call it foobar, and someone has a validator class with a protected function foobar, it'll break their code; the likelihood of this is lower, of course).

@samdark samdark modified the milestones: 2.0.11, 2.1.0 Dec 7, 2016
@samdark
Copy link
Member

samdark commented Dec 7, 2016

Need a changelog. Then ready to merge.

@cebe
Copy link
Member

cebe commented Dec 8, 2016

no need for a changelog, #11163 does already have one.

@cebe cebe self-assigned this Dec 8, 2016
@cebe cebe closed this in f5beaf3 Dec 8, 2016
@cebe
Copy link
Member

cebe commented Dec 8, 2016

merged, thank you!

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.

Add export of validation options without the js string
5 participants