🇨🇦Canada @Liam Morland

liam morland → created an issue.

Please put these changes into an issue fork and merge request.

Thanks!

Thanks!

Thanks for the patch. For the renamed method, there should be a deprecated method with the old name that calls the new name.

All phpcs tests now pass.

📌 Fix PHP 8.4 deprecation Active should be merged first.

liam morland → made their first commit to this issue’s fork.

liam morland → made their first commit to this issue’s fork.

Create a merge request with the patch in #31.

liam morland → made their first commit to this issue’s fork.

Looks good.

If you wanted to, you could do the ignores on a single line and have it ignore only the one phpcs rule:

public function op_contains($fulltext_field) {// phpcs:ignore Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps

The commit adding drupal:views_ui to test_dependencies was intended to fix this error: "Function views_ui_build_form_url not found." It does not fix it.

The other phpstan issues are all about adding dependency injection.

Re-rolled. Patches for 10.4.x and 11.x attached.

liam morland → created an issue.

The merge request is working for us in Billboard.

This patch is the current state of the merge request.

liam morland → made their first commit to this issue’s fork.

Minor updates made. Now coding standards pass and tests run, but they do not pass.

Why self:: instead of static::?

I think they all had default selected and I wanted to change the default. We got it working. I don't recall all the details, but we probably did some combination of manual changes and the three separate config imports.

Module "Mismatched entity and/or field definitions" (meaofd) → is useful for this.

Thanks for the patch.

Please make it pass coding standards.

I don't think there is any need to test for the existence of a variable before calling unset() on it.

Can you add a test for this?

Please put the change into an issue fork and merge request.

The order doesn't much matter. The commit message for this one need to be changed, though. The nullable types issue has already been fixed.

Please put your change into a merge request targetting 3.0.x.

liam morland → made their first commit to this issue’s fork.

Please put your patch into an issue fork and merge request.

liam morland → made their first commit to this issue’s fork.

Code comment improved.

liam morland → made their first commit to this issue’s fork.

Merge request created with the patch in #90.

liam morland → made their first commit to this issue’s fork.

liam morland → made their first commit to this issue’s fork.

I'm not currently working on the project that I needed this for, so I won't be able to write a test for it quickly.

For testing purposes, I have allowed entity_browser_block 2.0. I have not tested that this actually works.

It's trying to install on Drupal 11, but composer.json lists a dependency on a version of entity_browser_block that only runs on Drupal 10.

I prefer to fix the "Unsafe usage of new static()" issue by configuring an ignore rule → .

Please use an issue fork and merge request so that tests run.

I can see this being a useful improvement to the database API. There is CastSqlInterface, but it only supports one cast.

Addressed feedback and coding standards.

I don't use this any more, but if I will commit patches. Does this already work with entity_browser_block 2? Would it just take updating the require to "drupal/entity_browser_block": "^1.0 || ^2.0"?

I wonder why the automated compatibility issue has not appeared for Drupal 11.

Superseded by 📌 Automated Drupal 11 compatibility fixes for node_authlink Active .

Change is merged. Leaving open for more patches.

Since commit 6987c15 there is a .gitlab-ci.yml file so coding standards checks are getting run.

liam morland → created an issue.

I think it would make sense to commit this, even with some tests commented-out. The tests can be fixed in a follow-up. That way, going forward, other merge requests can be done on the basis of always having tests pass.

All tests are now passing in 📌 Make coding standards fixes Active .

Commit b5f9994 could cause a problem. Some people will have already run node_authlink_update_10203(). It is has been renumbered to node_authlink_update_10202(). So, if in the future another node_authlink_update_10203() is created, people who have already run node_authlink_update_10203() will not have it run.

The return might have been added to disable the tests following it. It may be that the tests only pass if some of them are disabled so this was done temporarily until the tests can be fixed.

You can use git blame to find out what commit added that line.

You could just have a single commit that adds use strict in places where it is needed. I suggest making a merge request that reverts the original commit. Then look at the eslint report and remove it only where eslint calls for it to be removed. Squash the changes on merge and you get a single commit neatly re-adding it where it is needed. Thanks

I grepped Drupal core for use strict and there are 117 JavaScript files that have it.

liam morland → created an issue.

Am I misunderstanding something about when 'use strict' is supposed to be in a JS file? I thought it was supposed to be there except in module files. Or put another way, it's supposed to be there unless eslint raises an error about it being there.

Drupal 7 is no longer supported. In any case, a report like this needs to include the file not just the line number.

Feel free to make coding standards fixes. That is what this issue is for.

Tests are not passing in Gitlab either. I suggest that there be an issue to get tests passing in Drupal 10. Once that is committed, then this issue can get them to pass in Drupal 11. I have taken steps towards this in 📌 Make coding standards fixes Active .

This has lead to 🐛 Declare dependency on jquery_ui in composer.json Active .

liam morland → created an issue.

I noted it missing from 5.2.x and since I didn't see a linked commit, I supposed it had not been committed. I see it was added in commit ac03fc27. I wonder why it doesn't appear here. Anyway, it's fine. This issue can go back to "Closed (fixed)".

The patch in #8.

Re-rolled merge request. All tests and checks are passing. This patch is the current state of the merge request.

I don't think this issue is needed anymore. The current latest commit on 6.0.x passes all tests, including on max PHP. It looks like everything was fixed in other issues.

It is best to test with the tip of the 6.3.x branch. You can check that out from Git or download the development version.

Sorry, I must have mis-read your post. Can you add your change to the merge request?

Does the problem appear in the latest development version of 6.3.x?

Please fix the coding standards issues. Why is the test change needed?

A markup change like this could make layout changes in cases of customization. So, I doubt this change will be made in 6.2.x.

I don't think there is going to be any more development of 8.x-2.x. Merge request 56 is for 3.0.x. This version needs tests to all pass.

I don't think there is going to be any more development of 8.x-2.x.

The error in #4 refers to code that is not part of migrate_tools. These changes look good to me.

As I said in 📌 Fix ESLint pipeline Active , the current eslint pipeline raises 23 errors like "'use strict' is unnecessary inside of modules", but this merge request removes use strict from 84 files. As I understand it, this should only be removed from the 23 files that are modules.

The "needs work" status is for when there is a patch attached to the issue, but the patch needs additional work before it should be reviewed.

🐛 When Track Changes is on, Link style module does't work Fixed is fixed. Does this problem still exist?

It's been more than a year. Presuming fixed.

Tests are now passing.

liam morland → created an issue.

The patch needs to be re-rolled.

I created a merge request with the patch in #2.

liam morland → made their first commit to this issue’s fork.

Rerolled

This issue is for fixing coding standards in the 4.x branch.

The "Implicitly marking parameter as nullable is deprecated" message has already been fixed in 4.x. The fix for it in 8.x-3.x is discussed in 🐛 Fix PHP 8.4.x deprecation and other warnings (PHPStan) Active .

tablefield 8.x-2.5 has been released. I expect this will be the last release for 8.x-2.x. 3.0.0 has also been released.

We shouldn't have to search for Drupal documentation or figure out from a list of search results which one is the version to refer to.

Please put your change into an issue fork and merge request.