-
Notifications
You must be signed in to change notification settings - Fork 225
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
API Deprecate renamed validator #1354
API Deprecate renamed validator #1354
Conversation
1164bea
to
bcdf137
Compare
public function __construct() | ||
{ | ||
Deprecation::noticeWithNoReplacment('5.4.0', 'Will be renamed to SilverStripe\\UserForms\\Form\\\UserFormsRequiredFieldsValidator', Deprecation::SCOPE_CLASS); | ||
parent::__construct(...func_get_args()); |
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.
RequiredFields constructor uses func_get_args() https://github.com/silverstripe/silverstripe-framework/blob/5/src/Forms/RequiredFields.php#L31
bcdf137
to
3be85ec
Compare
@@ -19,9 +20,17 @@ | |||
* | |||
* Required fields will be validated as usual. | |||
* Conditionally required fields will be validated IF the display rules are satisfied in the submitted dataset. | |||
* | |||
* @deprecated 5.4.0 Will be renamed to SilverStripe\UserForms\Form\\UserFormsRequiredFieldsValidator |
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.
* @deprecated 5.4.0 Will be renamed to SilverStripe\UserForms\Form\\UserFormsRequiredFieldsValidator | |
* @deprecated 5.4.0 Will be renamed to SilverStripe\UserForms\Form\UserFormsRequiredFieldsValidator |
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.
If the namespace isn't changing, we can probably just keep the class name? I can't think of another class we've only changed the name for and not the namespace though.
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.
Its easier to just have the FQCN. Makes it easier for things like automated tooling
3be85ec
to
5459312
Compare
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.
LGTM
Issue silverstripe/silverstripe-framework#10908