Merged, thanks Dave!
This hasn't really anything to do with project ownership transfer, which is only really used for abandoned modules if I'm not mistaken. But it for sure is related 🙂.
I'm following 📌 Add ability for organziations to manage/approve contributors(employees) Active so also related, but no the issue I had in the top of my mind 🙃 I'll check tomorrow if I can find it.
Do I believe we as a community have a right to overrule a maintainer: No.
That's 100% correct. And it's no difference from let's say maintaining an NPM package on GitHub. BUT I think we should at least try to find a way to make sure a maintainer is trustworthy before being added as a maintainer to a project. (see referenced META issue in the securitydrupalorg project).
Just. Going. To. Leave. This. Here. 🫥
https://www.drupal.org/project/issues/search?text=offering%20to%20mainta... →
Re #4
2. Are they part of a company on the community radar for concern?
I don't think that's really a trustable check. I can create a new account and register myself as an Acquia employee on Drupal.org. There is no check involved here. I remember seeing an issue about this somewhere to allow orgs to enable a setting that they have to verify users.
bramdriesen → created an issue.
Adding a few more issues. Think I have spotted 6 different modules so far where a user was added as maintainer without any second thought.
Spotted another user in one of the threads.
Adding TATA
bramdriesen → created an issue.
@legolasbo You might want to re-consider granting a random and new user maintainer access. This module has 10K installs, @peelas02 has no single post or contribution made for this module besides the D11 bot issue. This raises some security questions as well that this is being granted so easily.
Change looks straightforward enough! But should use $this->t()
I think
@g089h515r806 You might want to re-consider granting a random and new user maintainer access. This module has 35K installs, @peelas02 has no single post or contribution made for this module. This raises some security questions as well.
Adding #30 to the remaining taks. Something a site admin can do I presume.
I created a quick MR to fix the issue for our use case. What I noticed:
- When using a view (search API results) with AJAX, results are 0 when reaching the IF check
- The #view element is not showing all settings correct, E.g. AJAX is set to false while it is enabled on the view.
Thanks! I guess this could become a meta issue then. Created a follow up issue as the rendering stopped working on our site.
bramdriesen → created an issue.
@jradhak why are you not using merge requests/issue forks like you should? You're directly committing on the dev branch without any reference to the related issues. This way it's very difficult to track back the original issue.
Also, why are you dropping D9? I know it's EOL, but if there are no deprecations there is no real reason for that.
@jradhak Why are you committing changes to the dev branch before they are reviewed?
Why are you dropping D9?
Why are you not using merge requests but doing commits directly?
100% agree with what you said Catch
@deepali sardana Your MR introduces code sniff issues. It's not recommended to just blindly copy over what might fix a given issue from the issue summary. Also I don't see any need to wrap it in round brackets. I think the only thing relevant here is the string conversion, which might actually crash if the value is NULL. This might need a fix higher up.
But given your track record, you probably will not look again at this issue and just continue on the next one.
Fixing some meta data as I go.
I don't think the idea was to archive the DDEV WSL2 page. At least I don't see @catch or the IS mentioning that page.
I think tagging a new beta would also help to get some adoption.
Attempt at helping to clean this up. Also fixed a PHPCS issue on the last commit for the 11.x branch.
bramdriesen → changed the visibility of the branch 3498170-automatic-conversion-of to hidden.
bramdriesen → changed the visibility of the branch 3498170- to hidden.
bramdriesen → made their first commit to this issue’s fork.
xurizaemon → credited bramdriesen → .
It seems to reference "https://www.drupical.com/f/none.jpg" for all events actually, which means not even the placeholder is working 🙃
bramdriesen → created an issue.
For #13 and #11
Large images were also reported in #105 there it was suggested to convert to SVG which I think is a better approach. 🙂 But I ain't a front-end developer.
The logo got committed to the 2.x branch
Nice, only thing left would be to change the default branch to 2.x
Extending or using a badly named enum or class will not throw an error. This sniff only checks the declaration of the enum.
Correct, but fixing them would break things
Would also be quite disruptive in contrib space where on could be extending off a class or using an enum that is badly named.
Re #192 Note that I have nothing to do, at all, with the re-design or the development of it.
The argument that "it was announced at Barcelona" is reminiscent of Author Dent's house being demolished and the council claiming he had been informed because therr was a note in a locked file cabinet in a basement behind a door that said "do not enter".
....
comments that read basically as "you had you chance when didn't fell you".
You are pulling my comment out of context. My comment was as a reply to a comment that the redesign was "just" published without it being staged and without time for the community to respond and give feedback.
I have been doing Drupal News videos and paying very close attention to the developments of Starshot for thr past few motnhs and yet I missed that news entirely even after watching Dries's key note from Barcelone.
It was mentioned here with the preview link below it: https://youtu.be/nhPiL4g972A?si=VkXZoYImcGMZVOE9&t=1088
The brand redesign itself was apparently even shown in Portland (I did not watch that Driesnote 🙂).
@deepali sardana Like mentioned on many issues already to you. Patch workflows are deprecated. Please create a MR.
bramdriesen → changed the visibility of the branch 3496738-moving-moderationstatus-to to hidden.
Correct 🙂 a core maintainer can probably verify best if the current test is sufficient. But to me it looks to be the case. Latest test run is 100% green as wel after a random test failure.
So I think we can actually set this as RTBC.
Patch workflows are deprecated. Patch is introducing many PHPCS issues. Seems to be generated by AI judging by the sudden addition of inline comments which is typical for AI.
Also, is what is in the IS actually being fixed? I don't see any label related changes.
I have the feeling this MR will fundamentally break the module.
Ok, I actually think this is already being tested.
https://git.drupalcode.org/issue/drupal-3496195/-/blob/3496195-editor-mo...
Maybe we can just add a test (if it's not existing yet) that when we create a file using the Editor module, it ends up being permanent on the end. Not specifically checking if setPermanent is called twice, but just the end result.
Also sorry for the commit and not applying the code suggestion, totally missed that.
NW is more appropriate.
Quick review code wise, there are some things which are not correct. Also still needs tests, and currently tests failures.
MR looks clean. Just wondering if the "description" which was added in the test is really needed, unless it's required by default of course.
Test run looks good and the test only is failing as expected.
MR does not look okay. Started from the wrong branch? Wrong target branch?
We have failing tests, and still need new tests. The MR also introduces PHPCS issues.
Added a small nit, but will leave it at NR
And fixed :-)
Difficult to see between all the random failing tests. Triggered a re-run hoping to limit the amount failing tests so we can fix those.
Fixed the two remarks, English is not my native language 😇
Don't really think we need a CR as it was also internal before?
Updated IS and title, will set it back at RTBC as nothing changed in the code :-)
For the sake of avoiding further damage to the Drupal brand, please consider reverting to the original front page and moving this redesign exercise to a staging page where only those people who want to help make it better can access it.
That’s how it was before. The new UI was announced at DrupalCon Barcelona and was actually staged already a few weeks before that on new.drupal.org. Everyone was invited to test and post their constructive comments here.
Probably needs a small issue summary/title update
Fixed the @see to the relevant method.
Looks straightforward enough to me. RTBC
@deepali sardana, please stop making patches. You’ve been told this on like 10 issues already. Yet every new issue you start working on you create a patch.
Hi @deepali sardana
A few remarks:
- Patch workflows are deprecated, you should learn how to use the issue forks workflow. Especially with the GitLab issue migration around the corner. This will probably get you started:
https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →
also interesting is the DrupalPod workflow.
- Only make edits in scope of the issue. E.g. the changes to the node module or other tests do not belong here in this issue. I would limit the changes of this issue only to the phpdoc in StreamWrapperManagerInterface
. Changing to inheritdoc is already done in the other issue.
- I think you omitted some useful information from the code block.
1. fixed
2. test already refactored
3. test already refactored
4. test already refactored
5. Code doc already refactored
6. test already refactored
7. test already refactored
8. test already refactored
9. trait is refactored and no longer exists
10. fixed
So turns out this is only a minor change to the documentation in the default.settings.php
Looks ok to me. Still not easily readable though. Maybe converting '_' . $tries
to "_$tries"
helps, but I'll leave that in the middle for now.
Tests works as expected since the test only run failed.
Really not sure we can write a test for an interface. All tests are passing and code style is fixed.
bramdriesen → made their first commit to this issue’s fork.
Typo and added original participants.
bramdriesen → created an issue.
Code can still be improved. Nits from #23 are still not fixed.
bramdriesen → created an issue.
That was messy 😅 should be ok now!
bramdriesen → created an issue.
Adding partner levels.
I think that makes sense :)
Not sure why I left it on active.
This has not really anything to do with the community section but with the redesign.
https://www.drupal.org/project/drupalorg/issues/3475832 🐛 Feedback on Modern Drupal.org Design Active
Change looks simple enough :-) but also can't test.
bramdriesen → created an issue.
Thanks for testing and making the MR! Merged :)
Thanks @aylis that looks a lot better! Merged.
bramdriesen → created an issue.
To be honest, I find the documentation not sufficient. If I were new to the module and read
Alters the json response sent from status_dashboard_client.check.
I still have no clue as to what it's doing.
Also code nit, but one should not mix and match snakecase and camelcase in one file.