That's great, here's a static patch to be applied on composer
Just to keep things on track, here's a static patch with the fix.
murilohp → created an issue.
The #51 works great but it has some unnecessary files like .idea/php.xml, .idea/workspace.xml, .idea/vcs.xml, this new patch removes them making it easier to review.
I think the project is ready to be reviewed, moving to NR
murilohp → created an issue.
Added the gitlab ci template and fixed any remaining lint errors. Fixed
Just added a functional test to cover the settings form and the integration using drupal settings. Maybe in the future we could have another test.
Just uploading an static patch from the MR, after upgrading the core to 10.2, the hook update conflicted with an older one. If anyone else is having the same issue, I also had to update the hook update number in order to ensure the hooks will be executed.
FYI: drush ev "\Drupal::keyValue('system.schema')->set('system', 10100)";
I would just include some of that explanation in the similar modules section.
Good catch! Just updated the main page of the module adding what we discussed here.
Thank you!
Hey @w01f, first of all thank you for your interesting in this module, it's pretty new and it's awesome to have people testing and opening issues here.
Regarding your question, my idea was to create a simple module to just integrate adsense and Drupal through the script that adsense provides. This module does not have any blocks or further configurations besides the adsense id, the main adsense project that you asked, have a lot of features and provides blocks to configure where the ads would be placed. But since you can configure all the ads on the adsense admin panel, I thought it would be better for my projects to have just the simple integration and let my content editors to manage the ads on the adsense admin panel.
I think this will answer your questions, so I'll move this closed and if you have any further questions, please let me know!
murilohp → created an issue.
murilohp → created an issue.
Hey @divya.sejekan, thanks for your testing, but I don't know if this a problem here, you're gonna see this happening with almost all the icons.
For example: If my background is red, and I want to show the pinterest, the background color will conflict with the icon.
For this scenario you'll have to link another icon, and you can do it easily by accessing the admin/config/services/social-media
and changing the default image. So I'll move this back to NR, and here is another patch with the latest stuff from the MR.
Just updated the MR by removing any twitter references on the configurations and on the form, since I did this, we would need a hook update, so I have commited one, the following patch is just the current state of the MR since it's hard to patch the MR directly(poiting to a commit)
murilohp → made their first commit to this issue’s fork.
The 📌 Drupal 10 compatibility Needs review landed so I'm closing this one.
murilohp → created an issue.
Hey, although #3 is working we're facing the same issue after saving and cleaning the cache. So I'm providing a new patch that changes the route and fix this issue.
hchonov → credited murilohp → .
Just uploading a static patch to be used on a D10.1.
Uploading a new patch that checks if the variant label already exists on the page.
Hey @clarkssquared thanks for the testing, but I think this error has nothing to do with this issue, I suggest you to open another issue to address this error, apparently there's a call to this non exist service on the post save of the file: Drupal\page_manager_search\Entity\PageManagerSearch
. I'll move this back to NR to see what you think.
Here's a patch that makes the label field required, I don't know if it's the best solution but it's working properly right now
murilohp → created an issue.
Here's a patch with the proposed solution.
murilohp → created an issue.
Here's a new patch implementing an event subscriber, the old solution #4, wasn't working properly, now it should work correctly with the recent versions of drush on D10. This new solution is completely based on ✨ Ignore configuration storage collections? Needs work .
After a further investigation here, I found out an old custom module that creates a service that extends the schedulerManager, sorry for the noise here, the module is working as designed.
murilohp → created an issue.
Hey, I'm currently using #7 with version 2 of this module, since we're migrating the system to D10, I'm rerolling #7 to be compatible with D10.
Hey, just updating #4, there were some old jquery.once left on the js
Here's a patch implementing the proposed solution
murilohp → created an issue.
I had an specific scenario where the link_override__options
did not exist on menu_link_content_field_revision
table, so I've made this patch to skip the update when a field doesn't exist. I hope it helps.
Here's a patch with the proposed solution.
murilohp → created an issue.
Just uploading the MR as patch to properly fix a version and avoid any unexpected updates.
Uploading an static patch here since we cannot pin the MR and be able to use it.
Just uploading a patch here because we cannot pin the MR to be used in a project.
Uploading an static patch since we cannot pin a MR to be used.
Just uploading a static patch because we cannot pin the MR in a commit to use
Posting the static patch because using the MR doesn't allow pinning to a specific commit.
Just uploading a patch to keep things tracked in case the MR got updated
I was currently using the mr as a patch, but since it can be changed and affect sites on production without any announcements, it's better to use a patch.
Thanks @AndersTwo to open this issue, I'm also having issues with the drupal 10 upgrade, I'll update the description with the error that I found and I'll update the MR with the fix. I'm also moving this to NR.
Hey @mandclu, thanks for commiting this one, I was testing the module and I think there's one more fix left, according to the [#3180429], the field_widget_form_alter became deprecated and was removed on D10, here's a new patch with the correct hook.
Rerolling #3 to be compatible with the latest release.
Thanks @renatog, commited to 2.0.x.
Commited to 2.0.x, thanks
Thanks for your contribution @Kamlesh Kishor Jha! It looks good to me
We already have the 📌 Automated Drupal 10 compatibility fixes Fixed for D10, so I'm closing this one as duplicated.
I think this needs more information, I'm gonna postpone this one.
Hey @andypost I think you should go with the MR in this case. The MR have the test fixed(both for D9 and D10), and also incorporates the 📌 Deprecation warning 'Not marking service definitions as public is deprecated' Needs review (which can be closed as duplicated since the deprecation notice reported is for D10 and we can address this here), what do you think?
Just rerolling #45 to hal module 2.x
Commited to 2.0.x, thanks!
Thanks for your contribution! The patches and the MR looks good to me
This was commited on 7f8fcdd7, so closing as wont fix
Since we're moving to 2.0.x, this issue is not necessary anymore.
Commited to 2.0, thanks!
I was reviewing it here and it looks good!
The module was refactored on 2.0.x, moving this to closed.
The MR was merged, moving to fixed.
This was already merged, move to fixed.
Just closing this as fixed, commited on 2.0.x, thanks
This was fixed on 2.0, see #3346309: Update phpcs on 2.0.x →
Just closing this one to clean the issues queue.
Commited to 2.x, thanks
Thanks for you contribution @ilya.no! It makes a lot of sense, I was reviewing here and looks good to me.
This was fixed on the ^2 version, so closing this one.
Commited to 2.x thanks!
Thanks for the patch, I was reviewing here and it looks good, just changed the module_load_include('inc', 'onetrust_cookie_blocking', 'onetrust_cookie_blocking_js_list');
, I'll commit this and release a new D10 tag soon, thansk
Commited on 2.x, thanks!
For those who needs this on D10.0, here's a patch(basically what was commited on ef5c7f7)
Rerolling #6 to drupal10
Thank you for your input here @apaderno!
For these applications, we need a project where, in at least a branch, most (if not all the commits) have been done from the user who creates the application. In this case, I can only see a commit where your username appears.
Oh I thought I would be able to apply after fixing the issues, my bad here.
Verify the email used by Git is the same email associated to your account on https://www.drupal.org/user/3650571/edit → .
And thanks for pointing this, I haven't realized I was commiting using other e-mail
I think we can close this one as won't fix, since it doesn't make sense to apply for this role right now, I hope to get this in the future but not using this module, anyway, sorry for the noise here.
This will be needed to be ported to version ^2, meanwhile, the following patch will apply on ^1 and will be compatible with drupal 10
murilohp → created an issue.
Commited #2 to 2.0.x, thanks!
Commited to 2.0.x, thanks!
Thanks for your contribution @Jay Jangid, @arpitk and @urvashi_vora, I've reviewed the patch and looks good to me
I'm gonna close this issue, this type of functionality will not work on simple_sitemap 4 and it doesn't make sense to be here.
Adding a drush 11 compatibility and fixing some unit test errors