Account created on 30 October 2017, over 6 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3434566-automated-drupal-11 to hidden.

🇮🇳India ankitv18

I guess we can drop atleast Drupal 8 version.

🇮🇳India ankitv18

Indeed gitlab template is required further ~~ marking this RTBC.

🇮🇳India ankitv18

Please check below code: If we check at initial level of having a permission then only it could be useful for any further logic implementation.

/**
 * Implements hook_form_alter() for node form alter to add the Generator button.
 */
function metatag_ai_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if (\Drupal::currentUser()->hasPermission('administer metatag content')) {
     $form_object = $form_state->getFormObject();
    if ($form_object instanceof NodeForm) {
      $content_type = $form_object->getEntity()->getType();
      $selected_content_types = \Drupal::config('metatag_ai.content_settings')->get('metatag_ai.metadata_content_types');
      if (!empty($selected_content_types) && in_array($content_type, $selected_content_types)) {
          $form['actions']['generate_metadata'] = [
            '#type' => 'button',
            '#value' => t('Generate Metatag'),
            '#weight' => 4,
            '#ajax' => [
              'callback' => 'metatag_ai_generate_submit_form',
              'event' => 'click',
            ],
          ];
        }
      }
    }
  }
🇮🇳India ankitv18

Will add missing typehint to dataProviders method.

🇮🇳India ankitv18

Reviewed the MR and changes made are clearer and understandable in the statement and usage of cspell-ignore and disable is at appropriate place.
This looks good to be merged, hence marking this RTBC

🇮🇳India ankitv18

Considering the CSpell pipeline changes looks fine to be merged ~~ Marking this RTBC

🇮🇳India ankitv18

I've raised a MR!7, please review

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

drupal-off-canvas is deprecated
Follow the link: https://www.drupal.org/node/3305664 to support off-canvas dialog before and after D10.

🇮🇳India ankitv18

Marking this RTBC ~~ confirming it working as expected.
Clicking on the configure will redirect to the admin settings page.

🇮🇳India ankitv18

MR!36 can be reviewed ~~ included the changes of gitlab-ci from https://www.drupal.org/project/acquia_search/issues/3448516 📌 Automated Drupal 11 compatibility fixes Needs review

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

@itamair typedConfigManager passed as an argument from D10.2 in configFormBase class and settingsForm.php consists of config.factory and typed.config which is already passed as an argument in https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Co...

So as parent class already having those argument so I don't see any objective of repeating those property again and creating the instance with the same properties.

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

Will raise the MR considering the suggestion of #7 and patches provided

🇮🇳India ankitv18

Requires to update the tests especially for AjaxBehaviourTest.php file ~~ hence moving into needs work.

🇮🇳India ankitv18

Thanks @vensires apart from these failures https://git.drupalcode.org/issue/facets-3052574/-/jobs/1903753#L873
I've covered all fixes in the Drupal 11 compatibility issue.

🇮🇳India ankitv18

RTBC+1 to merge this one

🇮🇳India ankitv18

Assigning to myself to fix the warning in the minor and major phpunit.
By the way enabled the cspell job (Good to have to report the typo)
cc: mradcliffe

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

Noticed AjaxBehaviorTest phpunit tests are failing while working on the D11 compatibility issue against 2.0.x branch.
I believe tests should also be consider here to update them or provide new once. Also please rebase the MR so that gitlab pipeline would show the actual result.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

Finally pipeline passed ~~ included 9.5 || 10 || 11 and also fixed the cspell job.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3425236-drupal-11-compatibility to hidden.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3425236-search-api-ready-d11 to hidden.

🇮🇳India ankitv18

As search_api is released with D10.1 and D11 support, Rechecking if any work is left on this.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

Thanks @jcnventura,
I have dropped the D9 and used timeZoneHelper as you suggested.
Please review the MR and make it ease for @MegaChriz

🇮🇳India ankitv18

Ohh alright fair and square ~~ noted
Thanks :)

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

Yes @itamir
Sorry actually I just considered the MR feedback didn't take a look on the issue status.
Please review the MR!31 and let me know any changes required.

🇮🇳India ankitv18

Did some tweaks to fix cspell and phpcs pipeline ~~ marking this one RTBC

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3440317-fixes-for-remaining to hidden.

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

Updated the MR and enabled the next major pipeline, please review the MR!173
Created new issue to support book and laminas-feed tests and fix all deprecated method introduced in phpunit 10
https://www.drupal.org/project/feeds/issues/3454788 📌 PHPunit Next Major pipeline failure Active

cc: @MegaChriz

🇮🇳India ankitv18

Now pipeline is failing on https://git.drupalcode.org/issue/jsonapi_extras-3451984/-/jobs/1867677#L102
So I believe we can't run the tests below D10 cause of return type hint of public function normalize($object, $format = NULL, array $context = []): array|bool|string|int|float|null|\ArrayObject {
Dropping the D9 from info.yml, phpcs:ignore for comma trailing and previous major from gitlab

cc: @bbrala @ptmkenny

🇮🇳India ankitv18

for phpunit previous major: https://git.drupalcode.org/issue/jsonapi_extras-3451984/-/jobs/1867426#L217 (These are the failing)
Reason: to avoid phpcs issues like this: Drupal.Functions.MultiLineFunctionDeclaration.MissingTrailingComma
There are many places where comma separator is added at the end of closing statement in the constructor (These changes taken place couple of weeks ago).

So do we need that change or we can remove comma separator from those places and add phpcs:ignore Drupal.Functions.MultiLineFunctionDeclaration.MissingTrailingComma above last argument in the method.

cc: @bbrala

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

Updated the MR and fixes remaining type hint, validated on local by installing the module.

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

Rebased with 3.0.x branch and added the D11 in info.yml

🇮🇳India ankitv18

ankitv18 made their first commit to this issue’s fork.

🇮🇳India ankitv18

Rebased with 3.0.x branch to run the CI pipeline.

🇮🇳India ankitv18

@MegaChriz I've noticed you pushed logException Method few days back: https://git.drupalcode.org/project/feeds/-/merge_requests/127/diffs#a56d...
And going through the feeds.services.yml below logger is already there.

  logger.channel.feeds:
    parent: logger.channel_base
    arguments: ['feeds']

In this MR I used direct drupal call using \Drupal::logger('feeds')->error($e) instead of watchdog_exception
For PhpassHashedPassword deprecation I've replaced it with a https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Password%...

🇮🇳India ankitv18

There's already a separate issue where all the phpstan fixes are considered. Please check below issue
https://www.drupal.org/project/feeds/issues/3425218 🐛 Fix PHPStan errors Needs work

🇮🇳India ankitv18

Made the changes as requested Please review MR!59

🇮🇳India ankitv18

Created a MR with the patch#2 as it's a trivial change so would be better if its merged straight away.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch project-update-bot-only to hidden.

🇮🇳India ankitv18

PHPunit next major fatal error i.e

Fatal error: Declaration of Drupal\facets\Entity\Facet::__sleep() must be compatible with Drupal\Core\Config\Entity\ConfigEntityBase::__sleep(): array in /builds/issue/facets_pretty_paths-3430412/web/modules/contrib/facets/src/Entity/Facet.php on line 1213
/builds/issue/facets_pretty_paths-3430412/vendor/bin/phpunit:122

Is fixed in the https://git.drupalcode.org/project/facets/-/merge_requests/217/diffs#08e...

Rest all the pipelines are passing, hence marking this one as RTBC.

🇮🇳India ankitv18

MR!58 is ready for a review

Production build 0.69.0 2024