Inline unicode_requirements() in system_requirements()

I have brought the unicode_requirements() logic to system_requirements() in the patch.

@catch How to find out nid for "@see http://drupal.org/node/the-change-notice-nid" ?

Patch including deprecation notice.

BTW, the patch in #8 is also not correct. It still removes the old function, and puts the deprecation notice on the new function. Neither of those things are things that we want. :)

We need to leave unicode_requirements() in place, follow the full deprecation policy for it (reference: https://www.drupal.org/core/deprecation) and then (unfortunately, as above) copy the code and just inline it where it's called in system.install.

Thanks!

Oh, @Alka Kumari, for your question about how to get the node ID of the change record. Since we are adding the deprecation in this issue, we will need to create the change record here in this issue as well. Here's our handbook page on how to create change records for issues: https://www.drupal.org/contributor-tasks/draft-change-record

Re rolled #5 and fixed #6.

Thanks! A few more things.

1. +++ b/core/includes/unicode.inc
   @@ -10,7 +10,15 @@
   + * @deprecated in Drupal 8.0.0 and will be removed before Drupal 9.0.0.
   + * @see https://www.drupal.org/node/2764821
   

   It wasn't deprecated in 8.0.0; it's being deprecated for 8.4.0.

   Also, minor nit: There should be a blank line between the @deprecated and @see.

   Finally, this should include exactly the same text as the @trigger_error(), except with the documentation tags instead of words. See https://www.drupal.org/core/deprecation#how-function for an example.

2. +++ b/core/includes/unicode.inc
   @@ -10,7 +10,15 @@
   +  @trigger_error('Moves unicode_requirements() logic to system_requirements().@deprecated in Drupal 8.0.0 and will be removed before Drupal 9.0.0. @see https://www.drupal.org/node/2764821.', E_USER_DEPRECATED);
   

   So, there's a missing space after the period. But let's rewrite this whole paragraph to say:

   unicode_requirements() is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. There is no replacement; system_requirements() now includes the logic instead. See https://www.drupal.org/node/change-record-nid-not-issue-nid.

   Note how the wording for the @trigger_error() is slightly different, since it's a user-facing sentence rather than a docblock. You can see the format at: https://www.drupal.org/core/deprecation#how-function

3. Also, note above that we still need a change record and that should be linked instead of this issue node. See https://www.drupal.org/contributor-tasks/draft-change-record for instructions on writing a change record.

Also, @pk188, please be sure to provide an interdiff when you make updates to a patch. Thanks!

Here is an updated patch and interdiff.txt

This is for the first time , I am creating the change record.
URL : https://www.drupal.org/node/2884698

Let me know, if I am missing anything.

Thanks @Dinesh18, that looks pretty good. The change record is a good start also. There is one tiny formatting error in the patch:

+++ b/core/includes/unicode.inc
@@ -10,7 +10,16 @@
+ ¶

This line is missing its asterisk.

For reviewers, note that although the interdiff from #16 includes the patch file, the patch itself is correct.

I've added a bit more detail to the change record explaining why it's deprecated, and clarifying that the function itself should simply not be used (as system_requirements() also should not be called directly).

Thanks @xjm.
I will update the patch shortly.

Here is the updated patch which fix the missing asterisk.
I have reviewed the change record, it's much better than what I have wrote initially. :)
Thanks @xjm.

Alright great, I think this is ready! Thanks @Dinesh18 and @Alka Kumari.

@Calystod noticed that we linked the issue instead of the change record for this, so I've reverted the change to fix that since it was a mistake in the commit. Both @catch and I should have noticed this. :) We can just update the patch to link the CR.

New patch with the good link to the changes record page.

And the interdiff between patch 21 and 27.

It looks like the original patch linked the change record in the @trigger_error() but this issue in the docblock. The updated patch fixes that.

Back to RTBC, thanks @Calystod! Good catch.

Even after setting unicode , i am not able to store french characters. Getting database exception every time.