🇪🇸Spain @fjgarlin

The cleanup is now done. All ready for review.

Updated deployment steps as we no longer need the drush command.

Not a blocker for this, but I will clean up the code not needed.

If you see the conversations about, there is plenty of talk about the ddev plugin.

We removed it because it was no longer needed, everything is calculated beforehand or either via environment variables. The error is informing about what the problem is. I commented on the github issue, all it's needed is to remove the parameter.

That'd be ok for most contributors, though maintainers will get different behaviour when visiting contrib records from their projects vs other projects. I still prefer my suggestion because of this, but I know that it's just a preference so I'm not digging my heels in.

Your solution is easy to implement, so if that's the agreed way forward, I can do it.

We can do that, but I don't know if it'll be more confusing for users, as depending on the contribution record, you might find it open or closed.

Also, most maintainers will go there only to check that the attribution is correct rather than changing it, and in that case, collapsed would also make sense.

A mix between #3 and #4 would be to have it collapsed by defaul and add the "edit" link next to the person, which when clicked, will uncollapse the form.

@drumm - follow-up MR: https://git.drupalcode.org/project/drupalorg/-/merge_requests/414/diffs

This was the section that was assigning colors to the labels. We were fetching just the first page, and with this MR we will loop through all of them.

Merged.

fjgarlin → created an issue.

I went with the latter option:

It will be included in the next deployment to the site.

The first draft implementations of this had a ✔ for those cases.

The only caveat is that if we do this, then the "Git message" JS logic will no longer be dynamic. It will be set to the credited people and if logged out, you would not be able to interact with it. This is the main reason why the form was unified when we made it live.

Disabling the checkboxes should be the quickest/easiest, and we can put a "title" attribute prompting users to log in.

We could even hide it completely, and have a "Edit" button next to the contributor row, and when clicked, it would show the form.

This and the above are really doable, tho in both cases it will require an extra click.

A quick option I experimented with some time ago was to have the attribution form collapsed, like this:

Then it becomes obvious which save button belongs to which section. The logic of the two forms is really different so if something like this would work it'd be be real quick.

I'm going to put a "needs review" for this suggestion, and if accepted, I'll make an MR for it.

I don’t think we need a change record for this.

I will post on #gitlab and we can copy that message to the appropriate channels.

This is merged now! It's a great new feature.

I will tag it later today, make it the new default-ref and then announce it in the #gitlab channel. Thanks both!

Thanks for the screenshots. I think we can just wait until it happens. The output looks great.

> There is one @todo about swapping the URL when the new test is in a the next stable release. Were you thinking that could be done in a follow-up so we can merge this now? Or did you want to wait?
We can actually merge, and the "check-versions.php" job will actually warn us when there is a new release, and that's when we can swap that line. We'll need to remember, but that shouldn't be a problem.

I have given it a final pass at the MR, and it looks good to me. RTBC. Your additions since the previous RTBC are great!

That is correct. Removing those fields will likely make those disappear.

> Migrating this history for the new field would be a nontrivial task, nodechanges stores data as quite an unserialized blob.
This would require a lot more processing per comment, which will add to the total migration time. We'd need to build logic to process the nodechanges field into a sensible "markup".

> Does the migration to GitLab issues preserve nodechanges
No, it only uses nodechanges to detect when the issue was closed or when a file was uploaded, nothing else.

The main MR was merged. Any additional changes will be follow-up MRs, much smaller and easier to review.

From my point of view, things look good. They were good and all the work you've done have brought things to a much better place, better output, more consistency, and more flexibility.

Re the open threads, there is no action needed, just suggestions for namings and possible improvements. I'm happy to iterate a bit more or to leave things as they are.

Command to get the approximate number of issues per module:

drush sql-query "
  select fdfpmn.field_project_machine_name_value, count(*) as count
  from field_data_field_project fdfp
  inner join field_data_field_project_machine_name fdfpmn on fdfpmn.entity_id = fdfp.field_project_target_id
  where fdfp.bundle='project_issue' AND 
    fdfpmn.field_project_machine_name_value in (
      'conductor',
      'cdn',
      -- rest of module machine names from issue description
      'navigation_extra_tools'
    )
  group by fdfpmn.field_project_machine_name_value
  order by count, fdfpmn.field_project_machine_name_value;
"

Comments were never addressed. All CI issues were fixed in 🐛 CI fixes. Active .

Changing to a more correct status.

As mentioned, all of them can be easily fixed, except one, and we can use a phpstan ignore clause for that one. In fact, it is already done that way.https://git.drupalcode.org/project/config_notify/-/blob/8.x-1.x/src/Noti...

Your idea is great, but I'd only consider it for a possible 2.x version of the module, keeping the 8.x-1.x small and simple. Feel free to open the issue and work on it if you want.

As for this issue, phpstan job is clean https://git.drupalcode.org/project/config_notify/-/jobs/7145218, so I'm closing it as outdated.

Added contribution records endpoints and remove obsolete information.

Yeah, I've seen it a few times. Everything seems to work tho. Not sure if there is setting that we can do in "composer" to not report that, otherwise we can run the suggested command. We do it here https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/... for example.

Once done, we will probably need to update the documentation as well for external instances. See this commit: https://git.drupalcode.org/project/gitlab_templates/-/commit/39bf8a70cb5...

Agree with most suggestions, replied to the ones where I'm 50/50 on whether it needs to change or not.

Is there any generic open tool to help with this. Maybe using https://github.com/storybookjs/test-runner?

fjgarlin → created an issue.

fjgarlin → created an issue.

We’re doing extensive testing in staging and reviewing carefully the code of all the moving parts this and last week.

I can’t tell you an exact day but this is on the top of our list. We published yesterday a blog post with the upcoming changes: https://www.drupal.org/drupalorg/blog/gitlab-issue-migration-immediate-c... →

We will be verbose via blog posts and this issue about the progress.

Updated IS.

The migration has fully run now and it reports that all maintainers are up to date.

We have set our internal alarm so it goes to one of our internal slack channels, and we've put instructions about how to fix it if it happens again.

Creating/re-opening issue or a community slack ping is good as well.

Yes, there is a migraiton behind the scenes from D7 drupal.org to new.drupal.org

The migration was stuck as "Importing", even if it wasn't. I restarted it and it's running again, so it should be fixed within the hour. We are also setting some alerts for this internally.

Fixed one of the MR suggestions made in #13
Also fixed the issue with the theme in #15 (it was due to being a fork and not the canonical)
And also checked if it's a profile and then error in that case #16

I think all feedback is addressed now.

As ✨ Add recipes path handling Active was merged, I used the new variables created there, which made the code more simple.

It's ready for review. There are some debug lines which I'm not sure we should leave or just remove.

The "@todo" pending a new stable release from Drupal CMS remains. We could technically merge this as the "check versions" job will alert us when there is a new release.

My last feedback was a non-issue.

This is merged now. Thanks so much for all the work on this one!

I was doing a final review pre-merge and found a case where we'd have an error if there is no "tests" folder.

That's weird, I would have expected that to go to the vendor folder. Maybe drupal core composer plugin is moving it there.
But agree in any case that it's ok to have it on MR408.

I'll merge this later today or tomorrow.

I agree, but I think that what @drumm mentioned on #26 could (and should?) be the way to go: https://docs.gitlab.com/user/work_items/status/

Those could be made group specific and it will be a new "field", as opposed to a "label", which is probably the use that we want.

I think all labels can be made project-specific, even status.
We can promote them if needed: https://docs.gitlab.com/user/project/labels/#promote-a-project-label-to-...

All labels are now project specific. Should we need to promote them to group, that's possible:
https://docs.gitlab.com/user/project/labels/#promote-a-project-label-to-...

Code-wise, it looks complete for me. Testing happened in #42, #42, #46.

There is one case that @jonathan1055 wanted to check for the recipes installer path, but that doesn't need to block this issue.

Updated issue description with projects up to comment #55.

I don't think it will hurt having the extra installer path. In fact, if the dependencies from modules are recipes, it might actually be needed if we want the recipes to be in the correct/expected place.

Once ✨ Add recipes path handling Active is merged, we will have a better variable to use to detect if it's a recipe, which we can then use in the `script` section.

Also, we need to wait to release this until the next stable Drupal CMS is made, and ammend the "@todo" in the code.

Other than that, it seems to work with both modules and recipes and it caters for different install paths, etc.

Re #57, it'd be really easy to make most of them project-specific, it's just a matter of commenting out a couple of lines, as shown in the MR via a suggestion.

I will wait until this is fully agreed upon before making any changes on https://git.drupalcode.org/project/drupalorg/-/merge_requests/105.

Note that the D7 is still available: https://api.drupal.org/api/drupal/external_documentation%21developer%21t...

There was an issue with some branches a while ago where some pages were removed, but those should be back.

There are some errors on the branch applying, but I think those need to be fixed upstream.

I added docs, opt in variable, version and version checking. I also left some comments in the MR about some decisions.
Ready for review.

CI templates updated at 📌 11.3.x core branch created Fixed .

api.drupal.org only has 11.x (by design), so no action needed there.

Merged.

Yeah, I found it weird for it to happen so late in the week, but I guess since this was "just" the branch creation, it was less impactful than a stable release.

Merging shortly.

A lot of trial and error, but other than bringing module/recipe's dependencies, this is working. I added a @TODO for that.

The internal MR was merged.

I did a POC to test that caching works and it did. See this comment ✨ Enable cache Active .

I don't think that we need to change or document anything anywhere, as GitLab has good documentation about this. Core or contrib can start using this feature and report here if there are any issues.

RTBC (I guess it could be "Fixed" as well, but I'll wait until we have some more tests confirming it).

Upload to cache: https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/70...
Download from cache: https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/70...

This POC shows that it works.

I don't think we need to do anything about it, not even document it because GitLab has good documentation about it: https://docs.gitlab.com/ci/caching/

POC working for steps 1 to 3: https://git.drupalcode.org/project/config_notify/-/jobs/7006381

We need to do a "proper" setup to get "phpunit" to run and work, I just copy/pasted things.

That's weird, I checked the code briefly and according to this https://github.com/streetsidesoftware/cspell/blob/main/tools/perf-chart/..., and following on the defaults for the "error" function, the exit code should have been 1.

Merged. It will go with the next round of deployments.

Agree, it looks good. Most of the work was done previously when we swapped to use the other field, so I think this is all the clean up that is needed.

100%, we need to keep track of these things for sure. What they do affect all contrib and will continue to do so, so it's best if we can test these things on our test projects to help the rest of contrib.

Fixed here https://git.drupalcode.org/project/bluecheese/-/commit/f2a731d2dde675d0a...

I will go with the next deployment.

Updated the IS to include the recommended change then:

[#999999] task: Convert MediaSource plugin discovery to attributes

Co-authored-by: @sorlov
Co-authored-by: @quietone
Co-authored-by: @smustgrave
...

Not sure if the final agreement was Co-authored-by or just By tho.

I'd say let's create that case, or even a one-off MR, to be on the safe side.

There are some lines to be cleaned up in the MR too. NW based on that and the above test.

Thanks for fixing all these 11.3.x little things.

There are two steps for this:
- Create the menu entries (in the user menu, shown on desktop; and in the main menu, shown on mobile).
- Then hide the item when not authenticated. This might need to happen on "bluecheese_preprocess_menu" instead of here. This needs to be done this way because there is no "/dashboard" path on new.drupal.org, so the permission system is not checking this.

I will try to squeeze this in tomorrow.

Thanks for fixing this.

Really minor comments and replied to your question/suggestion. Needs work based on that. Great work!

Internal MR: https://gitlab.com/drupal-infrastructure/gitlab-runner-infra/-/merge_req...

--

Issue created: ✨ Enable cache Active
MR: https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/417

fjgarlin → created an issue.

fjgarlin → changed the visibility of the branch 3387117-test-s3-cache to hidden.

Internal MR rebased (still the same as #11).

Will create an MR here that tests the cache so we can see it in this fork/MR.

108 projects so far.

Updated the IS with the new ones (I hope I got them all).

Agree with #35, if all maintainers are not onboard this can be really disruptive.

I only took the first batch from #48.

I think the JS to rescue is here: https://git.drupalcode.org/project/drupalorg/-/commit/54fdf4bd59067dbf47...
(only part of it is needed)

...
$(document).ready(function () {
    var $body = $('body');

    // Comment attribution display. This is a global bind and must only be ran
    // once.
    $body.bind('click', function (e) {
      var $clicked = $('.attribution', $(e.target).filter('.attribution-label')).toggleClass('element-invisible');
      $('.attribution-label .attribution').not($clicked).addClass('element-invisible');
    })
    .bind('touchstart', function (e) {
      $('.attribution-label .attribution').not($('.attribution', $(e.target).filter('.attribution-label'))).addClass('element-invisible');
    })
    .bind('state:visible', function (e) {
      // Focus the newly-visible element, if the focus-on-visible class is set.
      // Wait a millisecond because the clicked element may want the focus.
      if (e.target.classList.contains('focus-on-visible')) {
        window.setTimeout(function () {e.target.focus();}, 1);
      }
    });
  });

...

I'll create an MR for this tomorrow.

So, the work made today is now on production. The migration is back in rotation, without the "sync" param, and the cleanup drush command is called right after it.

We also fixed an infra-related issue that was likely to affect this and other migrations.

Data seems to be stable for a few hours and a few runs, so I am going to tentatively close this again. I will monitor it in any case.
Thanks all for the patience here.

I made some small follow-up commits and I am testing the command in isolation first. It is working as expected.

Next step is to re-enter the migration without the "--sync" flag. I will do this after my break.

Re #3 "sorted by my most recent comment", that would be nearly impossible as comment information and timestamps will be in gitlab, and we are not syncing this information (nor there are plans to do it).

Adding another thread from a conversation with @xjm: https://drupal.slack.com/archives/C5ABFBZ5Z/p1761039428389919

Similar kind of data, so probably both things could be achieved.

--

*CR = Contribution Record

My suggestion:
- Filters: project / user / date range (required, default 3 months) / status (draft / (default) fixed)
- Columns: title (with link to CR) / project link / issue link / user link / attribution info (similar to how it's shown in the CR page).

We really need to check the performance of this as we have a high volume of data (+5 million rows and growing).

(adding this to the issue summary)

Removed the two instances of "please" and fixed a typo.

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

The bulk of the proposed solution is here: https://git.drupalcode.org/project/drupalorg_migrate/-/commit/d27b6efe9e...

The table is repopulated, I've removed the `--sync` option and I'm working on a more "custom" approach to remove entries that shouldn't be there.

The immediate issue is fixed now, so you should be able to grant credit again. I'm working now on the root cause, but that's behind-the-scenes work.

I thought of you during the week actually, hopefully we'll see each other next time.

We’re working on fixing this issue. Duplicate of the related one.

Yeah all data was truncated again overnight. Investigating.

Need to investigate wether we were hit but something like what's reported here:
- https://www.drupal.org/project/migrate_tools/issues/3104268 🐛 sync might be too strict during id comparison; rolls back everything Needs review
- https://www.drupal.org/project/migrate_tools/issues/3378047 🐛 drush migratation with --sync has suboptimal performance (v6) Active
- https://www.drupal.org/project/migrate_tools/issues/3110335 →

But I'll do that on a separate internal task.

Thanks for confirming. I needed to re-run the migration again and that made even more maintainers lose access to grant credits temporarily but the migration should be finishing again within the next 20 min.

Hi @lostcarpark, you should be able to issue credit on core now.

Webhook has been setup.

Good idea. In that case, it needs work.

We initially launched the widget with no-reply addresses, but then we got this one open 🐛 Authored-by should use email address from commits Active .

I think that the suggestion in #46 might be a good compromise.

We could have it open if the logged in user is a maintainer, closed otherwise, but I don't know if that'll make it more confusing. Having it always open might be just too much non-needed information in that block. So not sure how to go about that.

Missed some.

Updating issue description with projects submitted so far (it REALLY helps to symlink the project).

I thought about it (not closing) but then I also think that the actual copy vs symlink possibility, whilst small in code, it's a big set up change from how we started, so it's good step forward into a possible new set up, not needing to wait for a big refactoring.

I think that any further improvement could and should be captured, if wanted, in a follow-up issue.