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

Merge Requests

More

Recent comments

🇮🇳India ankitv18

Minor suggestion moving that line of change from constructor to Query function would work.

🇮🇳India ankitv18

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

🇮🇳India ankitv18

Marking this one outdated as this issue isn't replicated with latest release.
Feel free to open new issue if related issue encounter.

🇮🇳India ankitv18

I had tried to reproduce this issue at my local machine with the latest version of Key module along with other contributed modules like workbench_moderation, shield and encrypt on Drupal 11.1.5 application.

Can anyone else give a shot to check whether this issue still exists?

cc: @acbramley @rlhawk @q0rban

🇮🇳India ankitv18

Hey @joseph.olstad
Is this issue still exists as I tried with the below version:
Drupal: 11.1.5
Key: 1.20
smtp: 1.4.0
symfony_mailer: 1.5.0
symfony_mailer_lite: 2.0.2

Below are the screenshot of overriding smtp username with custom test key.

Am I missing something here? could you check with the latest version of modules and update the issue accordingly?

🇮🇳India ankitv18

So I've updated the MR ~~ now it consists changes in composer.json file and hook_update_N (to install acquia_dam).

🇮🇳India ankitv18

We should set a soft dependency, allowing users to uninstall media_acquiadam after successfully migrating assets.

Keeping acquia_dam in the info.yml prevents users from performing the uninstall action.

Also user needs to upgrade on latest version of media_acquiadam so by default they won't get acquia_dam module enabled and However, the migration process requires the acquia_dam module, which users can enable manually through the extend menu.

🇮🇳India ankitv18

Pipeline runs smoothly ~~~ No more warnings are there. Also, changes looks mergeable ~~ Hence moving into RTBC.

🇮🇳India ankitv18

Provided some suggestion ~~ also validated the functionality, it works as expected.

🇮🇳India ankitv18

Please add a hook_update_N to by default enable the Acquia DAM module so that user can see the migration dashboard fully functional.

🇮🇳India ankitv18

Ideally this should be the part of https://www.drupal.org/project/acquia_dam/issues/3515288 🐛 Newly created media type doesn't have media_library entity view display preconfigured. Active or if we are considering handling this separately then I've added the comment on the MR.

🇮🇳India ankitv18

The following files are responsible for handling the sorting and rendering of the asset count for dam_content_overview:

https://git.drupalcode.org/project/acquia_dam/-/blob/1.1.x/src/Plugin/vi...
https://git.drupalcode.org/project/acquia_dam/-/blob/1.1.x/src/Plugin/vi...

I attempted to debug this issue but was unsuccessful. I have attached a screenshot of the Xdebug for reference.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3447628-load-the-config to hidden.

🇮🇳India ankitv18

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

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3422332-fix-gitlab-pipeline to hidden.

🇮🇳India ankitv18

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

🇮🇳India ankitv18

Please rebase the MR so that CI jobs at least executed properly.
With referring #21 by @adamps if there's already a test then that need to be updated.

🇮🇳India ankitv18

Minor change required on MR.

🇮🇳India ankitv18

RTBC++
Faced similar issue with Drupal 11.1.3 version.
Fatal error: Declaration of Drupal\cors_ui\CorsUiCompilerPass::process(Symfony\Component\DependencyInjection\ContainerBuilder $container) must be compatible with Drupal\Core\DependencyInjection\Compiler\CorsCompilerPass::process(Symfony\Component\DependencyInjection\ContainerBuilder $container): void in /var/www/html/docroot/modules/contrib/cors_ui/src/CorsUiCompilerPass.php on line 18

🇮🇳India ankitv18

I've incorporate the changes with https://git.drupalcode.org/project/acquia_dam/-/merge_requests/113/diffs... (as this MR consists of Field swapping on media_type_presave hook with most of the logic using Asset, AssetDriver and Hook)

🇮🇳India ankitv18

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

🇮🇳India ankitv18

Looks good ~~ Marking this one RTBC

🇮🇳India ankitv18

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

🇮🇳India ankitv18

All green.. looks good to be merge.
Moving into RTBC.

🇮🇳India ankitv18

I would suggest to please wait for stable version of 1.1.x. There's lot of on-going development after RC-3 release.
And Thanks for reporting this issue.

🇮🇳India ankitv18

ankitv18 changed the visibility of the branch 3477335-include-phpcs.xml.dist to hidden.

🇮🇳India ankitv18

MR!55 is ready for a review.
@pfrenssen please check.

🇮🇳India ankitv18

Not sure why the MR is opened against 1.x branch the FR is for 2.x

🇮🇳India ankitv18

ESLint pipeline still needs attention ~~ few warnings left, please consider to fix them too.

🇮🇳India ankitv18

Looks good!! Hence moving into RTBC

🇮🇳India ankitv18

Thanks @ b_harold,
Now you can move into review and mark all threads resolved on the MR.

🇮🇳India ankitv18

I would like to suggest to add minimum support of D10.3 and then we can avoid using deprecationHelper or version_compare then things will remain intact.
@pfrenssen Any thoughts ?

🇮🇳India ankitv18

Left some comment, please check.
see@ https://www.drupal.org/node/3407994
You have to use version_compare to avoid this error.

🇮🇳India ankitv18

Looks good, hence moving into RTBC

🇮🇳India ankitv18

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

🇮🇳India ankitv18

Already Drupal11 compatible and Site studio version 8 support is available on 1.0.3

🇮🇳India ankitv18

Left few suggestions on the MR.
Keeping this in a NR state for other to provide inputs.

🇮🇳India ankitv18

Reviewed MR!117, left one comment ~~ rest looks good to me.

Keeping this in needs review so that other's can provide their inputs.

🇮🇳India ankitv18

Changes are updated and pipelines are passing.

🇮🇳India ankitv18

Hi @droath,
Thanks for merging this issue.
By the way you missed to give credits to the contributors.

🇮🇳India ankitv18

Hi @silverham,
As acquia_cms_headless module recommending ^1.3 as next-acms project needs to be updated with next-drupal v2 (Which is in a RC release). As soon as next-drupal npm package have v2 release we will update the next-acms project and release the new tag.

Thanks

🇮🇳India ankitv18

Hi @megachriz,
There are no new patches by projectBot so I guess we can mark this one fixed

Production build 0.71.5 2024