Rebased again to see if the patch applied to 11.0.0-beta1, but it does not. Uploading a patch file to use on the 11.0.x branch.
I think this may need a 2nd commit. This change only moved the libraries from core to the contrib modules, but it doesn't enable the modules. Which means that the newly added libraries are not enabled now, but the theme depends on them.
This may need the following changes:
* Make the theme depend on these modules in bootstrap.info.yml (see [#2937955])
* Create a hook_update_n() to enable these modules during install
* Also add the missing libraries from the jquery_ui module (see https://git.drupalcode.org/project/jquery_ui/-/blob/8.x-1.x/jquery_ui.li...):
** jquery_ui/widget
** jquery_ui/mouse
** jquery_ui/position
I tagged alpha3 a week ago. I think we can wait until a few more people test it with Drupal 11. Once Drupal 11 is released, then yes, it would make sense to tag 2.0.0.
Would also be nice if someone RTBCd that 🐛 Delete Login history records after user delete Needs review .
This fixed the theme for me in Drupal core 10.3.0. Please commit.
#176 doesn't apply to 10.3.x anymore, so uploading a new patch for use with that.
@programeta, @Ok4p1 and @HeikkiY the patch in #176 (and the MR) adds a "Hide untranslated custom menu links" configuration to the menu block that enables this functionality. #175 is a much simpler change that doesn't allow this to be configurable.
@sleitner merging the code for 11.x makes sense, but then you need to search for "11.0" and remove all the deprecations that were supposed to go in to Drupal 10.2, but will never be added.
No need to set this to "Needs work" if the workarounds in the issue summary are starting to get outdated. The workarounds are not really part of the issue, and are only there because of our failure to fix this in the last 10 years.
Looking at the update bot suggestion, I think we need to use TimeZoneFormHelper::getOptionsList() as we actually want to see the timezone labels in the form, and not only use the timezone identifiers.
Also, as per #16, let's make this simple and drop support for Drupal 9, and only support ^10.1 || ^11.
And a new re-roll.
Nice. Hopefully someone can also test and review it.
jcnventura → made their first commit to this issue’s fork.
@MegaChriz, your module and I do agree with not bumping version numbers needlessly. However, if you're introducing breaking changes, a major bump is justified. And for me, dropping support for Drupal 9 is one such BC break (assuming you're still going to do that as per #16).
Even if the MR is not yet dropping Drupal 9.3, it is a possible easy solution for my comments on the MR. The other possible solution (adding PHP 8 as a minimum) is also a BC-break.
@MegaChriz when you merge this, it might make sense to create a 4.x branch, and to tag a 4.0.0-alpha1 version when you add D11 support. This would allow you to keep adding changes to the 8.x-3.x branch in the case that something critical should happen, and to pivot Drupal 10 and 11 to fully use semantic versions.
Not changing the status, but I find it strange that there are so many unrelated changes to the phpstan-baseline.neon file, while the simple change done in this patch only shows up in the last 2 of the 5 blocks being deleted.
It won't go anywhere as long as the status is "Needs review".
OK. Reverted it. Thanks!
And yet, this line is in the MR:
"drush.services.yml": "^9 || ^10 || ^11 | ^12 || ^13"
Drush 13 will not support the drush.services.yml file (Drush 12 deprecated it). This shouldn't be merged like this.
I believe we can force the PHP version in the tests for this to be 7.4. Need to investigate further.
Yes, most likely this needs at least a composer.json with the correct require-dev, similarly to the D8+ composer.json.
Not much to review here...
jcnventura → created an issue.
You can see Drush compatibility here: https://www.drush.org./13.x/install/#drupal-compatibility
If you want to drop support for Drupal 9, you can choose to also only support Drush 12 and above. This would simplify the Drush support, as you can then simply drop the drush.services.yml file and move the Drush commands class file to /src/Drush/Commands. See https://www.drush.org./13.x/commands/
Nothing needing work. Unless you submitted a partial patch, and forgot the patch.
Setting to "Needs work" as per the MR comments in #42.
I remember the reason why I started having this problem was that the index key prefix length limit is 767 bytes for InnoDB tables using the default (COMPACT) row format. This 767 bytes quickly reduces to 191 UTF-8 characters. On the site where this occurred, I use views to quickly display several custom entities that have large text fields, and needed to create custom keys to speed up those queries, at the expense of a larger index file.
People running into similar problems that require the ability to change the row format should not have their hands tied by Drupal's inability to pass this parameter to the MySQL connector.
See:
* MySQL: https://dev.mysql.com/doc/refman/8.4/en/innodb-limits.html
* MariaDB: https://mariadb.com/kb/en/innodb-dynamic-row-format/
jcnventura → made their first commit to this issue’s fork.
I think that the change is actually good like that. Doing the replacement in the CR will force the .info.yml to be only "^10.1 || ^11". Either rector does that change in the .info.yml or it stays as is and rector can simply append "|| ^11" to the current core_version_requirement.
Please use DeprecationHelper in that system_time_zones() call.
The watchdog_exception can simply be changed to use Error::logException directly, as the Error class exists since Drupal 8.9 (see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Utility%2...)
Don't add drush.services.yml for Drush 13. Drush 12 has deprecated this and it will be removed in 13 (see https://www.drush.org./12.x/commands/ and https://www.drush.org./13.x/commands/).
Duplicate of 📌 Fix the issues reported by phpcs Needs review
Since this issue doesn't have an MR, I'd suggest to merge 📌 Fix the issues reported by phpcs Needs review and then check if there's still anything still outstanding that is fixed in this issue. A cursory look at the contents of both suggests that the fixes here are a subset of the ones in the other issue.
Checked all the changes in the MR. Only phpcs-style changes are being made, and the changes are good, The use of the null coalescing operator means that after this change the module is only for PHP 7.0+, but PHP 5.6 support has been dropped a long time ago by everyone (PHP, Drupal and all decent hosters).
Please add the GitLab CI changes, then merge this MR. We can then use the GitLab pipelines to see if there's anything still left. Maybe add credit on this issue to the ones that contributed to 📌 Drupal Coding Standards Issues | phpcs Needs review .
Looks good, and should test Drupal 9, 10 and 11. After merging this, please go to the "Automated testing" in the module's homepage and delete the Drupal CI tests.
4.0.x is an unused branch. It will probably be overwritten completely at some point. In any case, this MR is pointless.
I'd suggest dropping Drush 8 support independently if the maintainers want to drop support for Drupal 8 or not. No one should be using Drush 8 with Drupal 8.
Most users of Drupal 8 should probably be using Drush 10, so that the system can run on PHP 7. I don't think any hosting providers are still allowing PHP 5.6.
Please note that Drush 12 and 13 are VERY different beasts than 10 and 11..
Among others, I think drush.services.yml has been dropped, and the command files for Drush 12 should reside in src/Drush/Commands. Also Autowire should be used from Drush 12 onwards. See https://www.drush.org./12.x/commands/
Whatever has been committed, should probably pay attention to that, and limit usage of drush.services to Drush 9-11.
If it is really important to have padding set to 0, I need to see the before and after images that justify that change on the login form, and then there should be some effort to make sure that this only applies to the user-login-form.
Otherwise, please remove that padding change, as for now it is still too generic that it will also affect existing content.
Let's close this as outdated, as Nick asked this 11 years ago.
jcnventura → created an issue.
jcnventura → made their first commit to this issue’s fork.
If someone is able to read the code, and just switch the 1st if with the 2nd elseif, then no need to do two function_exists() checks.
Thanks!
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue. See original summary → .
Well, the broken tests, still need to be fixed, but CI was added in 📌 Add GitLab CI Fixed
jcnventura → created an issue. See original summary → .
jcnventura → created an issue.
We should focus any reviews of this in the related ✨ Allow multiple challenges Needs review
The implemention must be the reverse of the current state of the MR. The filterXss must be called ONLY during the setting of the configuration form. That way, no XSS is inserted into the configuration.
When retrieving the value, it is already XSS-free, so nothing needs to be escaped.
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue.
jcnventura → created an issue.
The base type of the entity log entity is from dynamic_entity_reference. It is not a trivial request.
I'll close this. No point in keeping this issue open unless someone first starts by coming up with an MR to do that.
jcnventura → created an issue.
jcnventura → created an issue. See original summary → .
To be honest, this is an awful coding standard. It would have been a good idea to avoid it a long time ago, but I'd rather disable phpcs on these lines than to do the changes proposed here.
jcnventura → created an issue. See original summary → .
Thanks everyone! And there were a lot of you. As to how to document, this.. I guess it will be part of the release notes when the next release is created. If someone would be kind enough to create a change record, that would be amazing.
jcnventura → created an issue.
Sorry for closing the MR, but it is easier to just run phpcs locally and fix all of the current coding standards, even the ones not reported here.