I think that "corepack" is modifying the existing "yarn" installation. In fact, if you see "yarn"'s installation instructions here https://yarnpkg.com/getting-started/install, it only requests "corepack"!
As you say, we might not know enough, and to be honest, we don't have to. We can just report what's happening. So I suggest adding _both_ versions of yarn, before and after, and saying "Yarn version after running 'corepack enable'", or something like that.
Setting to NW just based on this last suggestion.
From the composer.lock file:
{
"name": "drupal/webform",
"version": "6.2.9",
...
I know it's in two places now. We even considered removing it from the composer
stage altogether, but the time that we would save on that job might be multiplied in network requests in the other jobs requesting "node_modules".
Re versions, that's so strange! I wonder if it's due to the corepack enable
or the @gitlab-formatters/stylelint-formatter-gitlab
? Could we debug that?
We could add the version check before the if [ ! -d "$CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules" ]; then
(this is what we have now) and after that "if" is closed. That'd mean that it's due to corepack enable
. If the versions don't change, then we know it's the yarn add ...
line.
If it's the corepack enable
, then we'd probably want the "echo" lines after the "if", and not before.
I left comments and suggestions in the MR. I still prefer the simple check of whether the folder is there or not, regardless of variables or job names. We can still call the snippet in the "composer" job to get the versions output.
If setting the variables on the existing branch of the canonical project works I’d probably opt for that.
Relying on a fork might have more overhead down the line. Now it’s fresh but in a year’s time it might not be so fresh and be more confusing that having an extra (and documented) branch in the canonical project.
In any case, I’ll go with your call as you’re more familiar with the actual day to day and testing of the GTD pipelines.
Is that “bluehangar” project and exact clone of this template or is there any deviations from this?
Trying to see if adjusting the variable you mention is all it is needed.
Changes made in https://git.drupalcode.org/project/contribution_records/-/commit/aba2b96...
When a maintainer makes any changes to the attribution checkboxes, a message reminding them to save will be shown:
This is commited to the main branch and it will go with the next deployment, probably tomorrow Monday.
I placed the echoes in a way that they will always show, regardless of that variable setting.
This is great news! If the branch is going to stay, then I'd defo change the name to test-only-changes
, but I guess that's the plan.
Great work!
If we want the fix via code, please review https://git.drupalcode.org/project/drupalorg/-/merge_requests/400
Otherwise I guess we can tweak the block visibility settings.
We can see the output with the versions here https://git.drupalcode.org/project/keycdn/-/jobs/6516217
I'm working on a fix for contrib to add this feature, so please duplicate instead of moving the issue.
Note that you are linkinga Drupal core job output. The GitLab CI configuration for core is different that the one of contrib.
Having said that, this is a useful feature for contrib, which we can add, but if you want that added to core, you'll either need to move this issue to the Drupal core queue or create a separate one so it can be managed by core.
The issue seems to be here
$file = $this->entityTypeManager->getStorage('file')->load($row_data[$key]);
Where $row_data[$key]
does not contain the file id.
fjgarlin → changed the visibility of the branch 3546302-typo-in-banner to hidden.
The above MR is merged, which is great, but I found some small issues that still need addressing. I don't think they were hard blockers as we can use many variations which look good, so we can create new MRs with the amends.
I think that the text component or any other text elements in the Section components
paragraph need to inherit the Text color
setting from the wrapping Section container
.
See the three attached images to better understand what I mean. Light text is selected and the text cannot be seen in 2 out of the 3 background variations.
All user organizations are now migrated via the integrity check + drush command.
I guess that's where the logic to calculate BASELINE kicks in: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/...
One part is GitLab CI variables, the other (web) is calculated. Maybe we can add some verbosity there to see if that's the right command. Maybe add a git log before the calculation to see what's available.
I've integrated the above integrity check into our logs. We will monitor this from now on.
RTBC (not closed) while we monitor things.
That's a very clever use of the new system (adding comment to the SA, which will transfer the user to the contribution record via webhook), and we don't need to bring backa the old fields and code. Good one!
fjgarlin → changed the visibility of the branch 3545749-organisation-credit-counts to hidden.
This suggests that the summary is generated only from the first page of the pager.
That is correct. What's shown in that summary is the first page aggregated data, and the first page only includes 5 Drupal core credits.
We cannot get a full count per project without requesting all pages via http requests. The data is correct, it's just the way it's being rendered.
Maybe we can move from:
Documentation, 2 issues
Drupal core, 5 issues
To
Documentation issues
Drupal core issues
or
Documentation (fixed issues)
Drupal core (fixed issues)
And those will be links like this https://www.drupal.org/node/2373279/issue-credits/3060 → , where you can see all core issues fixed by Third and Grove.
This would be a quick formatting change, where we remove the innacurate number, and then we can focus the big efforts on 📌 Move issue credit listings to new.drupal.org Active .
We seem to be in the same situation 2 months after the setting was set to 50 (nearly two months).
Re-opening and setting the value to 0. I will regenerate the whole site again.
Thanks for reporting. The issue here is that the reported parts where it shows are views. So we might need to add a preprocess field hook or similar.
Thanks for the suggestion and MR, I think it's great. Setting to RTBC for now.
Oh, I totally missed that comment. Well spotted.
This was the last automated credit we had in the old system, as all other automations were removed.
If ✨ Simplify agreed commit format Active is done, this might not be needed.
It also has the same challenges as 🐛 Authored-by should use email address from commits Active , as commits only have email which may or may not lead to an actual d.o user. Having said that, this is just one user, so the logic would be easier.
Actually, postponing until agreed.
Before even starting on this, a format needs to be agreed on.
Linking related issue
✨
Simplify agreed commit format
Active
The MR above was merged.
The Commits API can return multiple commits per call.
And for each of those commits, we'd need to do an API call to search for the email (we can keep a map so we don't do duplicate calls). It's an API call for each individual email.
The "sync" button won't make any difference with the above calls as all of this (the message generation) is JS driven.
fjgarlin → created an issue.
I always struggle with these too 😅
Maybe just use an "if".
pseudocode:
a = '';
if (sa != '')
a = a + '|extra'
endif
This is now merged and it will be deployed shortly.
The subject and body were taken from the D7 version and changed slightly. Ready for review.
Added a @todo in here: https://git.drupalcode.org/project/contribution_records/-/blob/1.0.x/src...
The two current MRs as similar and against the same branch, which one should I review? (if any).
Sorry, I missed the "Needs review" here (really busy last 10 days...). Thanks for the work here!
This is deployed and should be fixed now. No need to resave defaults, it should just pick them up properly.
I think I found the issue. This commit shoudl fix it: https://git.drupalcode.org/project/contribution_records/-/commit/61cd04e...
Disregard the comment. Checking contribution records webhooks.
...were old issues, but that should not make any difference.
There is a difference with new issues vs old issues where you already are part of it. The setting should persist for brand new issues and new attributions.
But if you were already listed on an issue before September 3td, which is when we did the change, the value for that checkbox is the value that was set in the D7 issue.
We only use defaults when we are bringing a user for the first time into an issue.
I will close it with "Cannot reproduce", but if you notice this in new issues and new attributions, please reopen.
Note on emails 🐛 Authored-by should use email address from commits Active . Having the no-reply address is easy, possible and straightforward, and would not require any additional api calls. It was initially implemented that way until that issue was created.
Using the actual emails used in the commits (that's what that issue is about) would require an extra api call for every email listed in every MR and keeping a map of users to emails, and then replace those that were matched (which probably most of the time will be the initial no-reply suggestions).
It seems to also accept secondary addresses.
If we look for @catch's 3 email addresses listed in https://git.drupalcode.org/project/drupal/-/graphs/11.x?ref_type=heads
2 of them work, 1 doesn't. The one not matched seems to be automatically generated by gitlab and not set in a profile (I checked the same scenario with my user).
Going back to #8. We'd need to do an API call for every commit listed on every MR for an issue. Then get a map of email addresses per user and select the most recent.
@vitaliyb98 - I've run it for your user. Can you try again?
NW based on the missing automation.
I've added a new drush command to fix the inconsistencies detected by the integrity check that we added.
I'm going to run that command manually first on a few selected users and if it works as expected, I will add some automation so we can fix directly when detected.
fjgarlin → changed the visibility of the branch 1.0.x to hidden.
I'm integrating the new integrity script into our cron rotation.
I will follow up with a drush command to fix the mismatched data.
https://www.drupal.org/u/bbu23 → is also running into the same issue.
Note for myself: dev test link to debug https://fjgarlin-drupal.dev.devdrupal.org/node/3517801
Moving to the right project where we build that link.
Issues will be moving very soon to GitLab issues (the new Contribution Record was the final blocker to do this).
I would suggest against making a request to the new site to extract information.
I've updated both pages. Thanks for raising it!
On it.
@itamair - more and more actions related to code are being moved to GitLab. Merging MRs has been available for years on the GitLab UI and it is now the only place where MRs can be merged.
As the MR widget was heavily linked with the table of credits, it was removed so the merging workflow goes to GitLab. Issues for projects will be migrated soon.
Moving to the right project as the data is migrated with the wrong paragraph type.
I found this linked page: https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... →
Is there any other that needs to be updated?
I can see an issue with the synced data. I'm looking into this.
Clean up was deployed.
Thanks!!
Testing status change.
fjgarlin → created an issue.
fjgarlin → created an issue.
The above was merged.
Remove uneeded contributors block: https://git.drupalcode.org/project/drupalorg/-/merge_requests/397/diffs
fjgarlin → changed the visibility of the branch 3327584-automatic-message to hidden.
All the planned steps for "before" and "deploy" were done. The "after" clean up tasks will take place in the following days.
Number of comments yes. Number of commits no. We can also see if files were uploaded and reactions in MRs (eg: thumbs up)
Replied to the above via slack. Needed to be logged in and click on "Save".
So, what do you get in CHANGES_TO_REVERT
without the wrapping parentheses? Did you output that?
Also, https://www.shellcheck.net/wiki/SC2207 suggests that maybe adding some wrapping double quotes to the part inside the parentheses might fix the issue.
fjgarlin → created an issue.
Disregard comment. Fixing indexing issue.
Added more details to deployment steps.
Added a quick cleanup MR to the steps.
Re 2️⃣, got a +1 via slack from @nod_, @catch, @longwave, and I'll take @poker10 above comment as another +1.
Setting back to RTBC based on that and will wait for a final approval on this (aka mark it as "Fixed").
Note that the old format is still available in the Contribution Record page ("Not ready to switch...", at the bottom of the page).
I wouldn't like this issue to become as big as their predecessors, because here, it's just about simplifying the format due to the limitations explained in the issue description.
From the link given on #22, there is a neutral "Helped-by" which could also work well here. Tho there seems to be some buy-in for "Co-authored-by".
Given that the suggested format made it to RTBC in #6, and that most the conversation after was about changing "By" to "Co-authored-by" (there were also other questions that were replied), do we have consensus on the simplified format 2️⃣ suggested in the issue description?
This is merged now. Thanks.
avpaderno → credited fjgarlin → .