Nicaragua
Account created on 7 August 2011, over 14 years ago
#

Merge Requests

More

Recent comments

heddn Nicaragua

Posted feedback on the MR.

heddn Nicaragua

Commented on MR.

heddn Nicaragua

I wonder if 🐛 Fix backwards-incompatible Json data_parser API change included in 6.0.3. Needs work already fixed this issue. I've pushed some test coverage, but it doesn't trigger any errors. This might be a test only change to make sure these issues don't surface again. But I think the real problem is fixed elsewhere.

heddn Nicaragua

Is there any chance to add a quick test so this can't happen again?

heddn Nicaragua

Needs to be in an MR. Patches don't work with the workflow that triggers tests.

heddn Nicaragua

Commented on MR

heddn Nicaragua

Posted comment.

heddn Nicaragua

heddn created an issue.

heddn Nicaragua

Also, a more fundamental question. How is this different than config_entity that is part of drupal core itself?

heddn Nicaragua

If that isn't the case, we can re-open with steps to reproduce the issue on the current HEAD. Thanks for the confirmation this doesn't seem to be an issue.

heddn Nicaragua

Posted feedback on the MR.

heddn Nicaragua

Minor feedback posted on the MR.

heddn Nicaragua

There is one item on review that could be addressed.

heddn Nicaragua

Is there any chance of moving that patch into an MR and adding tests?

heddn Nicaragua

Drupal 10.1 is no longer supported. I think we can safely close this now?

heddn Nicaragua

I don't think this is still a valid issue any more. The code seems to pass all of the code standards test provided by the CI engine.

heddn Nicaragua

Thanks for the contribution here.

heddn Nicaragua

I'm relieved. Thank you for your blog post too. It will help others who are also facing this situation. Plus its a great walk through how to upgrade from annotations to attributes. I also found that using PHP Rector was very useful in this space too.

heddn Nicaragua

I've added https://www.drupal.org/node/3569238

We should add a test scenario to existing tests to test this new permissions model.

heddn Nicaragua

There was a quick followup in 🐛 Syntax error in DataParserPluginManager Active . Its landed for a while. I just noticed I hadn't tagged a new release though, so I've release it now with https://www.drupal.org/project/migrate_plus/releases/6.0.10

If you can still reproduce the issue after updating, let's open a new issue and link it back here so we can further investigate.

heddn Nicaragua

I read in https://www.undpaul.de/en/blog/2026/01/20/converting-php-class-annotatio... this is very disruptive and:

Old annotation-based plugins will not be discovered and will cause migration failures.

Is this truly the case? I thought that with listing Drupal 10.5 as minimum drupal core version and core's ability to support both annotation and attributes at the same time, that other custom or contrib modules could still maintain annotations without any more effort. Can anyone report to the contrary with steps to reproduce?

heddn Nicaragua

Thanks everyone for your long patience with this issue.

heddn Nicaragua

heddn created an issue.

heddn Nicaragua

Could we add a test

heddn Nicaragua

This makes me think we might have an issue if someone has the wrong permissions assigned. Any suggestions how to make this work in a BC manner?

heddn Nicaragua

Thanks for the contribution.

heddn Nicaragua

There aren't any deprecation warnings any more from this.

heddn Nicaragua

Comment posted.

heddn Nicaragua

Is there any way to get a test added for this? What is moderation dashboard that a test block or controller couldn't add so we can increase the scenarios tested for this module?

heddn Nicaragua

But the patch needs to be rolled into an MR to ever stand a hope of getting merged. We do that so it has tests run and is reviewable. Leaving at NW for this next task.

heddn Nicaragua

Thanks for that explanation.

heddn Nicaragua

There s a 2.0 stable release as of today. Is that an option now? Alternatively, we could backport this.

heddn Nicaragua

Thanks for the contributions

heddn Nicaragua

Can we roll into an MR so tests can trigger?

heddn Nicaragua

Thanks for the fixes.

heddn Nicaragua

This is not the case any longer.

heddn Nicaragua

We should add some doxygen about how to use this new feature. Otherwise, this is pretty much set to go.

heddn Nicaragua

Patch needs conversion into an MR. Also, is this an issue on 2.x? If it isn't then we can fix 1.x, but let's confirm it is or isn't there first.

heddn Nicaragua

Thanks for the contributions.

heddn Nicaragua

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

heddn Nicaragua

The patch needs to be rolled into an MR for tests to execute.

heddn Nicaragua

There's 2 competing MRs and no tests in either. Lets decide on which MR we want to use, close the other and ideally add some tests.

heddn Nicaragua

This should have tests.

heddn Nicaragua

This is now resolved. With passing tests, we can now tag a release.

heddn Nicaragua

Thanks for the contributions.

heddn Nicaragua

It seems to be some type of race condition. If I add a few sleeps into things, then it starts to collect. But it seems to collect a lot of noise from flood, session and previous user login that wasn't there previously. I'm not sure what the difference between functional and javascript is, but there seems to be something.

diff --git a/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php b/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
index d026b756d29..92adf9059c5 100644
--- a/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
+++ b/core/modules/system/tests/modules/performance_test/src/PerformanceDataCollector.php
@@ -81,6 +81,7 @@ public function destruct(): void {
     if (!$lock->acquire('performance_test')) {
       $lock->wait('performance_test', 10);
     }
+    sleep(2);
     $collection = \Drupal::keyValue('performance_test');
     $existing_data = $collection->get('performance_test_data') ?? [
       'database_events' => [],
diff --git a/core/tests/Drupal/Tests/PerformanceTestTrait.php b/core/tests/Drupal/Tests/PerformanceTestTrait.php
index 02872b84f48..04cb3ecce65 100644
--- a/core/tests/Drupal/Tests/PerformanceTestTrait.php
+++ b/core/tests/Drupal/Tests/PerformanceTestTrait.php
@@ -112,6 +112,7 @@ public function collectPerformanceData(callable $callable, ?string $service_name

     $session = $this->getSession();
     $collection->deleteAll();
+    sleep(15);
     $performance_data = new PerformanceData();
heddn Nicaragua

Thanks for the contributions.

heddn Nicaragua

Drupal 11 is now working, but Drupal 10 is now failing. Getting closer.

heddn Nicaragua

I'm confused. I've converted the JSON::API test over but the db collector isn't getting triggered to collect queries.

heddn Nicaragua

Any chance we can add a quick test case for this?

heddn Nicaragua

Can we roll this into an MR so tests can execute? Ideally we add tests.

heddn Nicaragua

I think this is long out of date. If that isn't the case, feel free to re-open and provide an updated direction.

heddn Nicaragua

I think this is long out of date. If that isn't the case, feel free to re-open and provide an updated direction.

heddn Nicaragua

Can we add tests?

heddn Nicaragua

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

heddn Nicaragua

Marking as duplicate of one or both of the mentioned tickets in #10. If this is inaccurate, feel free to re-open and document what else should be done.

heddn Nicaragua

The MR needs a rebase.

heddn Nicaragua

The latest changes in the MR are still failing tests.

heddn Nicaragua

There's not much we can do here. Except point to a blog post or a wiki page showing how to do this.

heddn Nicaragua

The patch should be rolled into an MR. Ideally we add test coverage.

heddn Nicaragua

The patch should be rolled into an MR. Ideally we add test coverage.

heddn Nicaragua

Those issues should be resolved fairly quickly, so I'm not going to move forward here unless those stall out for some reason. If they do stall, we can reopen and consider doing these changes.

heddn Nicaragua

The only worry I have with CI test failures is the cspell on dataparser. If we can add that to the allow list, then this looks good to me. The default previous version of drupal core is about to change any day now so I'm not too worried about getting that green.

heddn Nicaragua

Can we covert that patch into an MR so we can see how tests fair? And ideally add a test while working on this?

heddn Nicaragua

Thanks for rerolling this.

heddn Nicaragua

That makes sense. I guess if we wanted to we could add a test module with the legacy annotation and test for that. But not its fixed, it isn't likely to happen again.

heddn Nicaragua

Can we add tests for this scenario?

heddn Nicaragua

Can we add test coverage for this?

heddn Nicaragua

phpunit test failures

heddn Nicaragua

need to add a .cspell-project-words.txt with the names of the module maintainers to avoid cspell errors.

heddn Nicaragua

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

heddn Nicaragua

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

heddn Nicaragua

Thanks for the contributions here.

heddn Nicaragua

Posted some minor feedback on the PR. I did no manual testing.

heddn Nicaragua

If this is all about tracking a new release, updating the title.

heddn Nicaragua

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

heddn Nicaragua

There are tests failing now.

heddn Nicaragua

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

heddn Nicaragua

Any chance of this bug fix going back to 11.2 as well?

heddn Nicaragua

heddn created an issue.

heddn Nicaragua

I agree, this is a duplicate of https://www.drupal.org/project/layout_builder_st/issues/3497945 📌 Make coding standards fixes Active

heddn Nicaragua

https://www.drupal.org/project/layout_builder_st/issues/3497945 📌 Make coding standards fixes Active seems to have resolved this indeed.

heddn Nicaragua

Thanks for the improvements here.

heddn Nicaragua

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

heddn Nicaragua

Thanks for your kind fixes. Now, why didn't tests catch that?

heddn Nicaragua

is the thought to merge that over here?

heddn Nicaragua

Also, merge conflicts still seem to exist.

heddn Nicaragua

Posted feedback on the MR.

heddn Nicaragua

I assume that once this gets merged, we'll add the link target to https://www.drupal.org/node/3223395#s-migrate-drupal? Ah, just reviewed the IS. It is in there as next steps.

I don't see a CR though. Do we still need to add that? Otherwise, +1 on RTBC.

heddn Nicaragua

Needs a rebase.

heddn Nicaragua

Can we add some screenshots showing before/after the problem and its fix?

Production build 0.71.5 2024