Account created on 26 February 2013, over 12 years ago
#

Merge Requests

More

Recent comments

🇪🇸Spain fjgarlin

Webhook has been setup.

🇪🇸Spain fjgarlin

Good idea. In that case, it needs work.

🇪🇸Spain fjgarlin

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.

🇪🇸Spain fjgarlin

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.

🇪🇸Spain fjgarlin

Missed some.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

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.

🇪🇸Spain fjgarlin

Merged, thanks so much!

🇪🇸Spain fjgarlin

Yes, it was a temporary measure.

I did a follow-up commit and these are the two lines involved: https://git.drupalcode.org/project/api/-/blob/2.x/src/Formatter.php#L300...

We had some performance issues and we wanted to turn it off temporarily and see the impact as the queries run for this are really heavy on the database. It helped us identify yet another place where the load on the database is heavy.

We are planning on restoring this but we were waiting on DrupalCon Vienna to be over, so I'll probably bring this back next week.

🇪🇸Spain fjgarlin

Just rebased so CI would be happy again.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

I think this is good to merge. RTBC.

🇪🇸Spain fjgarlin

I think the doc suggestions look good as they are. We can always iterate if needed. Once you close that thread (with or without the suggested changes) I can review the whole MR.

🇪🇸Spain fjgarlin

I've merged this and I am deploying it today.

🇪🇸Spain fjgarlin

Update IS.

🇪🇸Spain fjgarlin

Created Create generic recipe "apply" code to test if a module is compatible with Drupal CMS Active for the recipes-only part of this task, but we can work on the other parts.

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

For composer:

composer create-project drupal/cms
composer require drupal/THE-MODULE

Then run the module's CI.

Recipes-only:
- The module needs to include a test to check compatibility with Drupal CMS.
- Kind of generic "apply recipe", which we will take from Drupal-CMS code and place it somewhere. (@phenaproxima TODO)
- If a recipe project has "that code", respect it.

🇪🇸Spain fjgarlin

Fine with leaving the "ls" inside the collapsed section.

As for copy-when-needed-symlink-otherwise, I'd rather keep it variable driven unless we really need to change it. Right now most contrib are totally ok with the symlink, which is useful for ddev (nice side effect), but we also get the added benefit of testing symlinked folders set up. The fact that it could be an issue for some projects might be a good thing to highlight somethng that could be done differently.

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

This is merged. Amazing work!!

🇪🇸Spain fjgarlin

MR ready. I will wait to deploy this until next week as I am at DrupalCon so I have limited availability.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

It looks good. I was about to RTBC but just left some really minor feedback.

🇪🇸Spain fjgarlin

This looks good. Thanks for addressing the feedback. RTBC.

🇪🇸Spain fjgarlin

That's great. Let's get the upstream issues worked on and merged this week (I'm at Vienna but they are small enough to be able to review and merge when ready).

🇪🇸Spain fjgarlin

I left some comments on the "verbose" issue, that can definitely go first, then this, and then we can tackle the "recipes" one.

🇪🇸Spain fjgarlin

Postponed until 📌 Discus adoption of #NoDiscard and PHP 8.5 Active is in a better state.

🇪🇸Spain fjgarlin

This is looking good. I left feedback on the other issue, so we can partly clean up all the verbose logic.

🇪🇸Spain fjgarlin

Answered your questions in the MR, one doesn't require action, the other does. NW based on that.

🇪🇸Spain fjgarlin

Note that the investigation is needed and important, but the "enforcing" of it might need to be discussed as some maintainers might actually want to run merge trains pipelines.

If we could put it behind a flag variable that we can turn on/off and document it'll be easier to move forward.

🇪🇸Spain fjgarlin

If you add _SHOW_ENVIRONMENT_VARIABLES: 1 to your .gitlab-ci.yml we can see all the variables that are at play in each pipeline and then compare differences.

🇪🇸Spain fjgarlin

Closing for now.

🇪🇸Spain fjgarlin

I can't replicate. What's the exact width of the screenshot?

🇪🇸Spain fjgarlin

Labels will be migrated like:

'version::' . $value
'component::' . $value
'priority::' . $value
'category::' . $value
'why::' . $value
'state::' . $value
🇪🇸Spain fjgarlin

We will start with some DA-owned projects first. We are currently testing things in our staging instance.

Re labels, we are not changing anything so far, we are migrating what we have as-is.

🇪🇸Spain fjgarlin

Even if Simplify agreed commit format Active changes slightly, it seems that this part will stay.

Are there any more suggestions that we could/should add or remove from https://git.drupalcode.org/project/contribution_records/-/merge_requests/12

🇪🇸Spain fjgarlin

Posponed until the format is fully agreed.
Simplify agreed commit format Active

🇪🇸Spain fjgarlin

Posponed until the format is fully agreed.
Simplify agreed commit format Active

🇪🇸Spain fjgarlin

We haven't had any new reports of issues around this area, so I'm marking this as Fixed.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

This is still managed in the 7.x-3.x branch.

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

Doing @gitlab_username is doable as we have that property stored in the user object. That'd be a relatively easy change.

🇪🇸Spain fjgarlin

For the d11-recipe branch, if we want to start with no tests, that'd be ok as well, we can add them at a later stage, as what we are doing now is the kind of first pass at supporting recipes.

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 2.x to hidden.

🇪🇸Spain fjgarlin

Great work! MR is merged.

I found an issue tho, but didn't want to block the whole MR for this.

See these images:

vs

So, for the drop-pattern-dark-blue background, the image borders are not visible. This can be addressed in a new MR.

🇪🇸Spain fjgarlin

You wouldn't get an issue notification if the credits are updated, the issue lives in D7, the credits in D10, and they won't trigger a notification just for saving them. You got the notification because there were actual changes in the issue made in October 5th by Benji.

The users are notified when they are added via the "credit others" field, and not in any other scenario. In this case, you created the issue, and you were added automatically to the contribution record, but no notification is expected.

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

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

🇪🇸Spain fjgarlin

I think all feedback is addressed now.

There is an overall WIP to make many of the UI elements ajax-driven, and that includes polling for branches too.

🇪🇸Spain fjgarlin

This issue would belong to the "core" queue.

But I'm not sure how useful this can be. If you look for " https://www.drupal.org/node/ " in core's codebase you can find thousands of references to issues. On 11.x:

$ grep -r 'https://www.drupal.org/node/' . | wc -l
2142

Some of those are references to issues, some of them to documentation pages, some of them to change records. Some of the references to closed issues are even intentional.

🇪🇸Spain fjgarlin

Add user as member at the same time that we are creating the fork: https://git.drupalcode.org/project/drupalorg/-/commit/a38e08d6cf7864cd5c...

🇪🇸Spain fjgarlin

Feedback addressed:
- DrupalOrgIssueForksQueueWorker::getAccessLevel() to be made a constant: https://git.drupalcode.org/project/drupalorg/-/commit/29790d172e9c5d330f...

Delaying logic: https://git.drupalcode.org/project/drupalorg/-/commit/25feba6992648d1dfb...
I created a second queue.
- We re-queue in the initial queue up to 20 times, after that we send the item to the "delayed queue"
- We check the delayed queue every 30 seconds. If the item was queued less than 30 seconds, ignore it, otherwise send it back to the original queue. This process has a max attempts limit as well.

I went for this instead of the sleep as requests could potentially queue and a bad/stuck one could affect others. I did not do a second rabbitmq queue because then we cannot use the delay exception implemented in the database queue.

The "delayed queue" is a generic one, so we can use it for any other queues where we need similar logic.

About doing more actions right after the fork creation, than can delay the feedback to the user. We can experiment with it once we have changed the logic to be more JS driven (big WIP).

🇪🇸Spain fjgarlin

Good feature request and great/simple implementation. Looks good!
RTBC.

🇪🇸Spain fjgarlin

Most of the logic is here:
- https://git.drupalcode.org/project/drupalorg/-/commit/1371edcd33192378fd...

Custom tracker is on the D7 script to migrate issues:
- https://git.drupalcode.org/project/drupalorg/-/merge_requests/105/diffs?...

I think this covers everything. Can you review @drumm?

🇪🇸Spain fjgarlin

fjgarlin changed the visibility of the branch 7.x-3.x to hidden.

🇪🇸Spain fjgarlin

Merged.

Will update Replace credit URLs with modern Drupal ones Active to use this filter.

🇪🇸Spain fjgarlin

📌 Use relative time ranges for filtering credit listings Active adds the option to use "months" as parameter, rather than the fixed timestamp. This will make linking issue listings for "last X months" always work.

I'll create an MR to adapt the links we added to use that parameter instead.

🇪🇸Spain fjgarlin

This is a good feature. I should have added it from the beginning since it is controller based.

I suggest we call the param months, make it "int" and it will just be "months ago".

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

There was a small bug that was undercounting results for the aggregated block on the modern Drupal side of things. Discovered by @pfey (already credited). The views were ok as they already had a similar fix.

Fix: https://git.drupalcode.org/project/contribution_records/-/commit/adcfee8...

🇪🇸Spain fjgarlin

This is ready for review. See test links in IS.

🇪🇸Spain fjgarlin

Added testing instructions.

🇪🇸Spain fjgarlin

Merged. It will go with the next deployment.

🇪🇸Spain fjgarlin

Updated links

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

It's probably inevitable that some names become obsolete. I'd go with d11-recipe for this one.

It's like all the old module naming convention 8.x-1.x... which is still valid for D11.

🇪🇸Spain fjgarlin

I added a check_plain to the MR.

🇪🇸Spain fjgarlin

I added some minor feedback in the MR. Things are looking really good.

🇪🇸Spain fjgarlin

That is good news. Thanks for reporting actually.

🇪🇸Spain fjgarlin

Great findings!! Thanks for investigating.

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

I wouldn't sed the string for valid characters. All we need here is just to trim both ends of the string of any spaces/line break characters.

I'd rather have a job to fail and break because the maintainer typed mod ule than pass and make the maintainer (or contributor) think that it's ok.

🇪🇸Spain fjgarlin

My idea here is really just detect all *.info.yml files, extract the core_version_requirement and if it's different, report it.

We don't need to do much I think, just a warning to make maintainers aware of this situation.

🇪🇸Spain fjgarlin

and wait for autocomplete

No need to wait, paste the three usernames (comma separated) and submit. Drupal should take care of the matching if I remember well.

🇪🇸Spain fjgarlin

The pipelines are happy with the change and it now tests D10 (with PHP8.1).

The issue we were having, in latest stable D10 and PHP8.1 was:

ParseError: syntax error, unexpected token "readonly", expecting "abstract" or "final" or "class" in Composer\Autoload\{closure}() (line 21 of /app/web/modules/contrib/mailchimp/src/ApiService.php).
🇪🇸Spain fjgarlin

final readonly class ApiService
That is a PHP8.2 only feature and it breaks Drupal sites running on PHP8.1. The PHP constraint could be indicated in composer.json to avoid this.

🇪🇸Spain fjgarlin

Doesn’t the credit others input cover that partially? You can paste there the list of usernames and save. You could have that list as part of the issue template so is quicker to copy.

🇪🇸Spain fjgarlin

fjgarlin created an issue.

🇪🇸Spain fjgarlin

Disregard comment. Fixing indexing issue.

🇪🇸Spain fjgarlin

Disregard comment. Fixing indexing issue.

🇪🇸Spain fjgarlin

Disregard comment. Fixing indexing issue.

🇪🇸Spain fjgarlin

Disregard comment. Fixing indexing issue.

🇪🇸Spain fjgarlin

https://docs.gitlab.com/api/project_integrations/#disable-a-custom-issue...
Needs to be on the D7 migration. We need to reset it so it's GitLab's one.

As for the rest. 100% correct. I've added links and todos to the code and I will work on it.

🇪🇸Spain fjgarlin

Support added in the previous commit.

🇪🇸Spain fjgarlin

I had a few changes in progress and I added this as part of those. See the commit https://git.drupalcode.org/project/drupalorg/-/commit/a2dfaae63f1933b6cb.... The trait function is gone and all usages were changed to the user service function.

Production build 0.71.5 2024