🇳🇱Netherlands @bbrala

Yeah I'm also leaning towards a separate job for preparing the caches. Although it might cost you a little in feedback after commit.

He actually did the icon to be skipped in the channel. So nothing to see there.

🐛 Browser language detection is not cache aware Needs work has been updated to use a responsesubscriber that allows setting Vary headers. Wonder if those changes also need partial split into this issue?

Well at least everything is green. So thats good. But it some weirdness with the event subscriber. Not sure how that should work. I didn't investigate the redirects and such. Mostly just refactored the current recent patch to work with an event subscriber.

All green :)

One of the mentioned issues is 🐛 Browser language detection is not cache aware Needs work . Did some work there to see if that can actually get fixed sometime soon.

I played around with this and moved the implementation to a eventsubscriber as asked for by @znerol. I see one things that is very weird though as per comment in the code:

    /**
     *  Either Drupal\Tests\page_cache\Functional\PageCacheVaryTest::testPageCacheWithVary
     *  fails or Drupal\Tests\language\Functional\LanguageBrowserDetectionAcceptLanguageTest::testAcceptLanguageEmptyDefault
     *  fails if you remove one of those. Not sure why
     */
    $events[KernelEvents::RESPONSE][] = ['onRespond', -100];
    $events[KernelEvents::RESPONSE][] = ['onRespond', 100];
    return $events;

Not sure why, those priorities sometimes confuse me a little.

bbrala → changed the visibility of the branch 2430335-browser-language-detection to active.

Isnt the fallback pretty much automatic, since if its not available it will just run without?

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we'll open threads for them as appropriate.

2️⃣ Celebrations :tada:

3️⃣ What’s the recommended way if you have a module and mark it D11 that has a dependency on another that has not.

4️⃣ Handling update path divergence between 11.x and 10.x

5️⃣ New site module compatibility checker tool (for contrib projects used on sites) (edited) 

6️⃣ Drupal 11.0.0-beta2 needs help!

Participants:

smustgrave, moshe, mikelutz (he/him), mark_fullmer (he/him), xjm, Gábor Hojtsy, VladimirAus, Kristen Pol, catch, mstrelan, bbrala

0️⃣ Who is here today? Comment in the thread below to introduce yourself.

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we'll open threads for them as appropriate.

2️⃣ Celebrations :tada:

3️⃣ If you've gotten a wrong Update Project Bot patch, here are some threads you may want to look at...

4️⃣ Review needed on 📌 Hardcode security coverage EOL dates for Drupal 10.last-1 and 10.last Needs review

5️⃣ If you are running into unexpected newer deprecations, this thread could be helpful...

6️⃣ Help with must have issues for 11.0.0-beta2: 📌 [meta] Requirements for tagging 11.0.0-beta2 Active

7️⃣ Should we have regular weekly updates from the Project Update Working Group in this meeting?

8️⃣ Issue needs review: ✨ Get rid of jQuery in displace event Needs review

9️⃣ Work needed: ✨ Replace dialog positioning with floating-ui Active

1️⃣ 0️⃣ Organized D11 documentation updates?

1️⃣ 1️⃣ Thanks for attending the initiative meeting :+1: Please feel free to continue the conversation in the threads or start a new (numbered) topic below this... threads are "officially" open for 24 hours to provide async collaboration in all time zones

Participants:

Kristen Pol, japerry, Fathima Asmat, finnsky, smustgrave, VladimirAus, bbrala, Gábor Hojtsy, tekNorah

0️⃣ Who is here today? Comment in the thread below to introduce yourself.

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we’ll open threads for them as appropriate.

2️⃣ Celebrations :tada:

3️⃣ RC1 was originally planned for this week, what's the expected timeline?

4️⃣ How can people help to get RC1 out? 🙂

Participants:

mikelutz (he/him), hestenet, Fathima Asmat, xjm, VladimirAus, Kristen Pol, Gábor Hojtsy, catch, benjifisher

0️⃣ Who is here today? Comment in the thread below to introduce yourself.

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we’ll open threads for them as appropriate.

2️⃣ Celebrations :tada:

3️⃣ 📌 [meta] Requirements for tagging 11.0.0-beta2 Active With D10 keeping support for longer, messaging around upgrades will become broken when new minors get released (via @xjm) (edited) 

4️⃣ 📌 [meta] Requirements for tagging 11.0.0-beta2 Active Drupal 11.0.0.-rc1 has been released, anything that needs doing?

5️⃣ Shall we start an issue for the Drupal 11 release landing page?

6️⃣ Drupal 11 documentation. Should we start separate guides for each major version or keep current ones up to date?

Participants:

bbrala, xjm, dww, catch, kim.pepper, VladimirAus, quietone, Kristen Pol, hestenet, Gábor Hojtsy, andypost

I looked at using artifacte: when: and indeed, seems pretty hard to get it to do the work there.

I specific cache job is a possibility, but thinking about this, this kinda is a bandaid until we can just use real caches by gitlab? I ended up not caring to much if it is seperate or not. Its a little more controlled seperately which is good. Might make the move to gitlab cache a little easier since we only need to change very little in the main job.

I end up with preferring a seperate job. But the preference is kinda light.

Also rebased. :x

Hmm, digging into my memory i think the following reasoning was important here. I've gone through this with @casey to see if what you say is true. We've updated the comments in the tests and adjusted the test with 1600 to 801 to hopefully be more clear on the intention of the tests.

To awnser your comment: Cache entries track expire time, the cache record is valid in the second it invalidates. See cache backends:

$cache->valid = $cache->expire == Cache::PERMANENT || $cache->expire >= $this->time->getRequestTime();

The cache should be renewed the next second after expiring. A max-age of 0 means the cache is still valid but will not be next second. Invaliding the cache is still just done in the cache backend through the expire data. We are not changing anything on that layer. The only thing the max-age does is tell the layers in front of drupal (nginx, a CDN or whatever) it is about to expire.

Should we disable the cache on max-age 0 by returning FALSE in the ResourceObjectNormalizationCacher::get method we should pretty much get the same data because the cache has not expired yet.

Whats important to note is that the test only tests the ResourceObjectNormalizationCacher and its logic around max-age. We are not testing the cache backend and handling of expire data. We assume that is correct. (although i didn't find any tests around the expire=0 and handling of that case).

Most important though, how does the doc page need to look. Not sure about that. But if we have a direction on that adding is should be easy.

Commenting because this might be kinda annoying if we want to get stuff into core.

Wonder how this should actually look in that page. The parsing is probably not that hard. But we might need: https://github.com/phpstan/phpdoc-parser which adds support for a lot of extra types? (see https://github.com/phpstan/phpdoc-parser/blob/fcaefacf2d5c417e928405b71b...)

You could possible scope the caching with usage of the variables of gitlab and add it to the phpstan job id think. That menas no extra job, but only cache on commit.

Interesting to just do it manually... Keeping an eye on this one.

Just a note. I did not handle the changes in the tests as discussed in #24

Rebased and refactored a lot, and did all the feedback.

Unfortunately i ended up in a testfailure in this test:

    // Check that translations cannot be created for non-translatable nodes.
    $node = $this->drupalCreateNode();
    $output = $this->doTestNewTranslationRequest($node, 'it', Response::HTTP_UNPROCESSABLE_ENTITY);
    $this->assertSame('Translation is not enabled for the specified resource.', $output['errors'][0]['detail']);

This should be the result of refactoring the checks on enities to a check on the resourcetype. Might be because it not uses the entity type instead of the entity itself in checkEntityTranslatability (now renamed to checkResourceTypeTranslatability).

Time for today is up unfortunately.

As mentioned, please use slack for this interactive discussion. If we end up with some good best practices there, we might be able to document that. But closing this for now since is has been open for quite a while now.

When i go to the endpoin for user in jsonapi i see:

"attributes": {
        "display_name": "bjorn",
        "drupal_internal__uid": 1,
        "langcode": "en",
        "preferred_langcode": "en",
        "preferred_admin_langcode": null,
        "name": "bjorn",
        "mail": "bjorn@swis.nl",
        "timezone": "Europe/Amsterdam",
        "status": true,
        "created": "2024-07-18T14:18:42+00:00",
        "changed": "2024-07-18T14:19:07+00:00",
        "access": "2024-07-19T12:12:14+00:00",
        "login": "2024-07-18T14:19:07+00:00",
        "init": "bjorn@swis.nl",
        "default_langcode": true
      },

So it seems this is outdated? Is that correct?

I wonder if this has been improved enough with this issue: 📌 Improve JSON:API test failure messages to include errors when data is expected Fixed .

Errors are now a lot better on failures showing the actually errors instead of the generate "missing expected 'data' "

This was talked about in #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC → .

General consensus is dat we don't see the worth of this change. By reference is not really used. Also docblocks are getting less important with typing systems of php getting better over time.

For that reason I'm closing this issue as wont fix.

This issue was discussed in #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC → .

We seem to be positive on this, but the IS needs some love.

Also mentioned was this rule: https://github.com/slevomat/coding-standard/issues/1684

This still needs documentation updates.

You are right, not sure how i missed that.

The change did not so as per IS, remove the whole todo.

Moved all changed to 11.x, also updates a return type that didnt make sense. I think this is good like this. I've updated the issue summary also.

I still think this is the best option for adjusting, even though this could mean you enable and disable multiple times. But i don't think this will be an actually issue in implementations.

I really think this is the way, it might need some priority juggling to make work in your application but that should be fine. I'll update the MR to 11.x and make sure codestyle and everything is up to date.

Seems all good. Ty.

larowlan → credited bbrala → .

They have been added to the dev dependencies to make gitlab work. So closing this one.

Drupal 11 compatiblity and Gitlab CI

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

Manually tested on 11 and 10. All good :)

Manually tested on 11 and 10. All good :)

The three calls are ceil and round. Which means we are not running into the problem that (int) cast will always round down. Changes are clear, output is correct. RTBC for me.

Rebased onto 11.x and fixed a small issue with a missing return type.

quietone → credited bbrala → .

0️⃣ Who is here today? Comment in the thread below to introduce yourself.

1️⃣ Do you have suggested topics you are looking to discuss? Post in this thread and we'll open threads for them as appropriate.

2️⃣ Celebrations :tada:

3️⃣ RC1 was expected last week but was postponed to this week

4️⃣ "Handling update path divergence between 11.x and 10.x" is outstanding for RC1

6️⃣ Release notes for RC1 needs more work

7️⃣ For Drupal 11.1(!) add alpha level package manager to core

Participants:

hestenet, Gábor Hojtsy, mikelutz (he/him), Kristen Pol, longwave, VladimirAus, andypost, catch, Spokje

bbrala → created an issue. See original summary → .

Sorry, assumed the issue move was enough.

drupaltechtalk →

This issue will not be fixed. If you want to help out check out this issue: https://www.drupal.org/project/admin_toolbar_messages/issues/3460980 📌 Drupal 11 compatiblity and gitlab Needs review

Oh wow, good call. I think we should actually be able to do that.

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

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

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

All green! :D

Did drop d9 support because of issue with traioling commas. But meh.

bbrala → created an issue.

All green :)

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

Closing and removing bot tag/

Closing and removing bot tag/

All green, added gitlab also.

Keeping active for new issues from the bot if needed

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

bbrala → created an issue.

All green :D

All green, think we are fine :)

bbrala → created an issue.

bbrala → created an issue.

Thank you. But will not be merging this, we do not use comments in this way. I'll put in gitlab and phpcs in a d11 issue.

Sorry, but already working on this :)

bbrala → created an issue.

All good, agreed :) Moving to proper queue.

Also updated the 3.x issue with the same changes, then upgrading wont be too hard for users: 📌 Automated Drupal 11 compatibility fixes for paragraphs_edit Needs review

bbrala → changed the visibility of the branch 3460956-d11-compatibility to hidden.

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

This seems fine, perhaps we should merge ✨ Drupal 11 compatibility and gitlab Needs review first and get that into 3.0 to get gitlab running also for phpstan/cs validation.

Tryinig to get through all issues.

Should the permission migration be different? Wouldnt the user need like content edit permsissions, the authenticated role seems a little optimistic, this could also be a user that can just login to comment or something?

I know it is failing, but that is next major since paragraphs is not yet compatible with d11. Declaring compatibility is fine since we this modules is not holding back d11.

Added gitlab yaml with phpstan which will help with developement, did do a few fixes for codestle :)

bbrala → created an issue.

Make opt-in settings copy-pastable.

I think this is fixed now. Problem was indeed the difference of `include=` versus no include paramter at all. I would love a review on this, perhaps test this patch on your site.