Also, every contribution record page had a link to this issue, in the part about the old format:
I'd say that the first part of the commit message is now compliant with the conventional commits format (linked in the contribution record pages). I cannot foresee the future, but I think this (small) change is an improvement, and the first part fully adheres to the standard.
All downstream pipelines ran: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/657773
The failed jobs are as expected.
For now, I applied the suggestion. Happy to change to the singular form if that's what's agreed.
The versions between one and the other are very different: https://git.drupalcode.org/project/coder/-/compare/8.3.7...8.3.31?from_p...
That would explain the difference between packages, I guess. But not recognizing the standard "Drupal" is a whole different thing. No idea why that happens.
I'm happy applying the suggestion, but I asked a question before I do it, to be on the safe side.
I'm happy with the code as it is now. So "Needs review" for real 🙂
Re #16, I was confused. Maybe the re-run took the current version of the code, instead of the one of the commit being tested? What I mean by that is that the get-file-via-curl.sh takes the code from the tip of the branch. I was doing/undoing things so that would explain it.
Found the reason and fixed it.
Forced error: https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72... and https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
For both D7 and D10
No errors in here https://git.drupalcode.org/project/gitlab_templates/-/pipelines/657505
Forced the error in here https://git.drupalcode.org/project/gitlab_templates/-/pipelines/657511 but the error wasn't captured or shown
I gave it a go at it. Please review the solution.
https://git.drupalcode.org/project/gitlab_templates/-/pipelines/657483 has a D7 working pipeline and a D10 one.
Yeah, that's a good point. We know the failures, so we'd easily spot new ones.
I'll make sure to run all downstream pipelines before merging anything from now on.
Well, the idea is that no project will lose any issue, at all. They will just be moved/redirected to GitLab issues.
Happy for GTD to cover the PHP5.6, that's why api-d7 is still there to be honest.
No worries. Supporting PHP5.6 is a kind of edge case at this point, and we have the follow-up issue 🐛 Symlink project script needs to be PHP5.6 compatible Needs work for it.
We could fix the typo before the other issue gets merged in if you want, but if you want to focus on pushing that forward, I'm fine with that too.
I found an issue, but it's unrelated to this work. I opened it here: 🐛 Symlink project script needs to be PHP5.6 compatible Needs work .
I will merge this to main shortly.
Maybe we can use PHP_VERSION_ID to run that code or not or to have different versions of it.
We can perhaps write a version of the code that doesn't use $rest_index and uses the length of the $options array to slice the array.
This code was added in ✨ Add --verbose parameter to symlink_project.php, and a variable turn on verbose Active
I re-ran many other downstream pipelines, and they all run as expected (including the failed jobs).
I've gone through all the changes again, and it looks really good. Mostly, it is the variable calculation and then the replacements all over. I think it's well tested and documented. RTBC.
Right now I'm just trying to keep the projects in the same order as they arrive, but that doesn't mean we will process them that way.
We won't know how many we will do until we start migrating some and see the results. I guess at first we will be more cautious with the first batch, and as the code is refined, we will do more and more.
It's a 1-line MR, I'll do it so it's easy to either merge or iterate on.
The changes are now implemented (commit) and deployed
As this is a core issue and I am not a core maintainer, I won't close it nor assign credit (not all reported users participated). I will leave those tasks to core maintainers.
1. Yes, let's change the docs to use the new variable where appropriate.
2. Happy with that. It should be an easy change at this point.
Adding some of our own DA projects:
-
https://www.drupal.org/project/drupalorg_drupalcon_theme →
(6 issues)
-
https://www.drupal.org/project/drupalorg_migrate →
(29 issues)
-
https://www.drupal.org/project/contribution_records →
(38 issues)
-
https://www.drupal.org/project/drupalorg_crosssite →
(117 issues)
-
https://www.drupal.org/project/gitlab_templates →
(430 issues)
"drupalorg" and "insfrastructure" have almost ~4k and ~5k issues, so not considering those for this initial round.
📌 Decouple issues from www.drupal.org issues (parent and related) Needs review was merged and deployed now.
Only pending deployment step is to remove the issue cockpit from the project's page.
Then we can start migrating projects from the opt-in issue (+200 projects).
I can implement the agreed changes for this as it is RTBC.
As the changes would happen in the "contribution_records" module, should I move this issue there or just leave it here as it is a "core" decision? I am going to do the latter (implement the changes but leave the issue here), but if you feel like the issue needs to move just do it.
I will also wait a few days of the issue being RTBC to make sure the status is not changed back.
Thanks all for the back-and-forth and suggestions, and eventual agreement.
Merged. It will go with the next deployment. Thanks all for the back-and-forth.
Not sure about this one because it’s better that all the logic of each file is completely inside the file, as these are curl’ed into the jobs and separating the logic would make the process way more complex.
I think it’s just coincidental that we have such function duplicated as the cspell and composer php scripts don’t have any overlap.
Unless a good and clean solution is proposed this might be a “won’t change”.
Good suggestion. Validation added for issues and change records link fields.
We can completely hide or disable instead, to have less show/hide elements (we already have messages showing on the credits form).
First load:
After a change is made:
This behaviour is per-form.
The following MR contains the above changes. Please review.
When you make a change, then reveal the button and a “remember to save!” message.
We already have this for the credits table form.
I do like the idea of not showing the save buttons on both forms until changes are made. I will work on something for this.
fjgarlin → changed the visibility of the branch 3556541-attempting-to-create to hidden.
I suggest using the composer command to get this value in the first place rather than analyzing content of files.
Something like this composer config --list | grep bin-dir should give us the output that we need, and it won't matter if it was set up on the file or is the default, it will just output the value.
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.
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;
"
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?
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.
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.
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.