๐Ÿ‡ฎ๐Ÿ‡ณIndia @ankitsingh0188

Pune
Account created on 23 July 2015, over 9 years ago
#

Merge Requests

Recent comments

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

The $value type can be array or string or bool most of the times it would be array but we are not sure if the type is bool then in that we need to change it to array|string|bool

IMO, we can use mixed return type instead of specifying the each return type individually.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ changed the visibility of the branch 3414535-wrong-type-in to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave We are not sure whether it'll be array or string or int or bool.

The mixed type accepts every value. It is equivalent to the union type object|resource|array|string|float|int|bool|null.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave - Please review

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

git checkout -b '11.x' --track drupal-3414535/'11.x'

The above commands not working because 11.x branch was already exists while taking the clone from drupal.git

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

Conflicts resolved, please review the MR!

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ changed the visibility of the branch 11.x to active.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ changed the visibility of the branch 11.x to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

I contributed to DrupalCamp Pune 2024 and handled the responsibility of Photography #dcp2024

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

Not sure the test is required. Let's leave it for the next reviewer.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

Please review the child issue https://www.drupal.org/project/drupal/issues/3380781 ๐Ÿ“Œ Rename $op with BC usage in update.php Active

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave - I think there's no tests earlier for this file. Do we really need a test for this. Please suggest.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave - Please review my latest commit and let me know if there's anything else needs to be updated.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

ankitsingh0188 โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

There's already a code in "hook_user_login" which will validate if there's a destination query param and it's value should not be '/user/login' then will skip the redirection.

/**
 * Implements hook_user_login().
 */
function redirect_after_login_user_login($account) {
  $current_route = Drupal::routeMatch()->getRouteName();
  $request = Drupal::request();
  if ($request->getRequestFormat() !== 'html') {
    return;
  }
  $destination = $request->query->get('destination');
  if ($destination && $destination != '/user/login') {
    return;
  }
๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@jungle I have updated the patch and using IoC wherever possible.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

What if we get the DatabaseException ?

In that case $e->getMessage() will not work as mentioned Steps to reproducesection.

I think the exception interface should extend the \Throwable with respect to what you're catching is something that is a \Throwable.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

I've created the patch with the new CR.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

Hi @murilohp

it's better if you add the interdiff between #24 & #29.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave Still needs a change record, which would then need to be added to the patch. -> Please help me with this I have added the tests and refactor the whole interface also, if there's anything else missing please let me know.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

@smustgrave what changes exactly can you please elaborate?

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

All the patches are same only the difference of the file path as patch was failed to apply earlier. I have included the interdiff in #10

Additionally the patches are failing in #11 but I ran the retest again and it successfully passed.

If you see the error youโ€™ll get to know due to date mismatch test failed just because after 12 midnight day changes.

๐Ÿ‡ฎ๐Ÿ‡ณIndia ankitsingh0188 Pune

Patch #24 ๐Ÿ› Notice: Undefined index: type in Drupal\Core\Render\Element\Date::processDate() Closed: outdated applied successfully. I have tested the patch and it works for me.

I have attached the screenshot before_patch.png in which I am getting the notice and it disappears after I applied the patch.

Looks good to me! Good to move it to RTBC.

Production build 0.71.5 2024