🇳🇿New Zealand @quietone

Converted to MR and fixed more occurrences.

This has been postponed on a Coder issue 🐛 Drupal.Commenting.VariableComment.VarOrder - @var before @code Needs review . After working on this today I commented over there 🐛 Drupal.Commenting.VariableComment.VarOrder - @var before @code Needs review that I think there is nothing to fix. That is, the sniff is fine as is. One of the points in that issue was that an @link/@endlink before an @var was an error. There happens to be no such usage in core, so although it could happen it isn't. It seems like an edge case and should not block enabling this sniff.

Therefore I am setting this to NR.

I worked on the core issue 📌 Fix 'Drupal.Commenting.VariableComment.VarOrder' coding standard Postponed today and reporting back here.

The Drupal coding standards page api-documentation-and-comment-standards#order does not mention @code in the order of sections. Should it be considered a section?

Can make an in issue in the Coding Standards project so that can be discussed.

Core does not have any '@copyright' or '@license' in doc blocks. So, that point seems to be outdated.

And '@link .. @endlink' is reported as an error. But I think this is correct for the Variable comment block where the explanation is after @var.

Also, there were no instances in Drupal core where an '@link .. @endlink' was moved as a result of enabling the sniff.

So, for me I would go with won't fix on this.

Changing this to fixing the violations that are currently ignored by // phpcs:ignore Drupal.NamingConventions.ValidVariableName.LowerCamelName

No longer postponed. I rebased onto 11.x

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

quietone → created an issue.

Removed completed issues

Only two more issue to complete!

And no longer a child of the core issue.

Adding what this is postponed on to the remaining tasks per Remining tasks → .

Adding what this is postponed on to the remaining tasks per Remining tasks → .

Rebase onto 11.x.

quietone → changed the visibility of the branch 3063877-add-access-control to hidden.

I restored the test.

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

Failing commit code check locally only. PHPStan doesn't like it when there is only one file and it *.api.php.

@threeg, anyone can update the proposed resolution to reflect what has been agreed.. Having that up to date is very useful for reviewers and committers to confirm that the implementation matches the community agreement.

I read the IS, comments and MR. I updated the proposed resolution and changed 2 comments in the MR to use email and not e-mail according to our standards, See https://www.drupal.org/drupalorg/style-guide/content → . The MR changes are minor, so leaving at RTBC.

While this make sense I do wonder about sites that don't have an update email address and don't realize it. Maybe they need a reminder?

Does this also fix, 🐛 If notifications.email is a string, an error occurs Needs work ?

There is a comment about adding a followup issue. Is that needed? Adding tag just in case.

This is tagged for an accessibility review. That still needs to happen. I am not sure of the process but asking in #accessibility is a start.

Closed some issues that I think are duplicates, no credit to transfer. Also hiding files.

I think this is a duplicate of 🐛 Allow attributes to be passed to the password confirm fields Needs work , therefor closing.

There is a later issue that has more discussion and work on this problem. I am closing this as a duplicate.

The issue in #2 refers to all attributes, therefore I am closing this as a duplicate of that one.

@sukr_s, thanks for working on an older issue!

I read the issue summary, comments and the MR. There are no unanswered questions.

I left a comment in the MR asking for a comment to be changed. The title here needs to be updated because it refers to 'password' but the MR makes no changes to password, only to password_confirm. Both of those should be simple to complete.

No it should not fail, it is independent of the system.install. The test is relying on the downgrade_test module to provide the hook and that module is only installed in the test after proving the requirement failure. The test is a functional test of the concept. There is no similar test using the actual system_update_11101 because I don't see a way to prevent the update process from finding and executing that hook.

Trying for a better title. On scanning I kept thinking this was a regression.

@Berdir, thanks for the review. I responded to all the comments.

Good point about the novice tag, so I will remove it.

quietone → changed the visibility of the branch 3094865-fix-references-to to active.

quietone → changed the visibility of the branch 3094865-fix-references-to to hidden.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

I read the issue summary, comments and the MR. I see there are no tests, but that is explained in #19. I have not evaluated that with the testing heuristics. A reminder to us all to summary Slack discussions in a comment. There is no indication here why $value is not an array or if that is really a contrib problem.

Also, trying for a better title. Leaving at RTBC.

I read the issue summary, which is incomplete, there is no proposed resolution. And as, an issue that affects the UI there should be screenshots available from the issue summary. I am restoring the template and will update as I continue. Thanks you to andregp for summarizing the relevant changes and what comments have been addressed.

I think everything has been addressed here, but worth checking that #72 🐛 User's image in comments overlaps with Layout Builder section RTBC is complete. Also an Olivero maintainer commented in #72 that they are "indifferent" about this being changed.

Leaving at RTBC

I have been thinking about this a fair bit, sparked by recently spending way too many hours to get some documentation to display correctly on api.drupal.org. Once that task was done, the next thing I did was look at what Symfony does. The reStructuredText (RST) format certainly looks much better for writing example code than what we have now. As for prose, I don't think there is as much of a difference, but still RST is easier. I did note that in the Symfony docs one can't view a single php file.

For the proposals

1. Doing this is more likely to lead to improvements than the longer path of creating an issue, MR, and so.

2. What I think about here are, in no order, a) how the docs will be structured, b) need to follow a page of instructions to get setup to contribute, c) who has commit access, d) may not improve the documentation, e) adds a new place to find documentation without removing one. But I do think this should be pursued. And proposal 1 can happen independently of the decision on this.

And with both 1 and 2 we don't have to wait 1 week for api.drupal.org to be re-parsed for the doc to be updated.

I read the IS summary, the comments and the MR. The proposed resolution is out of dateAll questions are answered and the threads in the MR are correctly resolved.

I then applied the diff and tested. This works as expected and is an improvement!

I do think there follow up work.

• Add this feature to media files as well.
• The warning message uses the string 'It is advised' which is the first occurrence in core for a warning message. Because of that I think that the usability folks should review the string.
• The first time I read "It is advised to store file uploads for contact forms as private files. You can configure this in settings.php" I read the first as something to action so I tried to. Of course, the option is not available and I knew that but I still tried! So, again, I think the a usability review would help.

I think the above items can be done in followups. I am adding the tag for that.

I finally read the test. Is there a reason it is only testing 1 code path?

I read the IS and the comments. Thanks for the up to date issue summary. There are no unanswered questions but there are changes to core/modules/views/tests/src/Kernel/Entity/RowEntityRenderersTest.php that alexpott said are not necessary. They are still in the MR. And then, looking at the MR I left a question about changes to another file.

I read the change record and it is easy to follow. It is good that includes what sites should do. Nice work. At first I thought the title could just say that the link is not displayed instead of 'restricts'. But maybe that is a good thing so people are curious and read the notice.

Setting to NW for the 2 comments in the MR.

I read the IS and comments. This is changing the UI, so adding tag.

I applied the diff and it works as expected, after clearing caches. I see that the after image in the issue summary does not match what I see in testing. In my testing '(navigation)' is '(Navigation)'. The latter is consistent with the other usages in the block list. And because of the consistency I am not tagging for a UX review.

I think this is a bug because it affects the functionality of the site for admins and developers.

Leaving at RTBC

I read the IS, comments and MR. There are no unanswered questions. My question about the tests in #22 was answered but then later the tests were combined. That seems OK in this case.
Leaving at RTBC.

According to #5 this has been addressed and there has been no discussion in 5 years to indicate otherwise. Therefore, closing as outdated. If that is wrong, add a comment and set the status to active.

The second method is documented in the Drupal deprecation policy → . The first method, as is pointed out is not suited for core, where something is either to be deprecated or not. Therefor, I don't think there is anything to do here, certainly not for core.

Only did a rebase and some docs updates because of request in Slack. I did notice there are more methods that need documentation, that declare strict types hasn't been added and that not all method arguments have type hints. I can't help with that now.

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

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

I've skimmed through the Issue Summary and comments and I think this is now outdated. The testing environment on DrupalCI with has evolved quite a bit since this was opened. For one, the linting was done as part of the testing. And most recently, Drupal completed the move to testing with GitLabCI. The Drupal wiki has more information about Automated testing. → .

Closing as outdated. If that is wrong, add a comment and set the status to 'active'.

@hinikato Thank you for the idea!

There has been no discussion here and no support for the change. The proposal doesn't met the Criteria for evaluating proposed changes → . In this case, there is not demonstrated demand and support for the change.

Moving component to where the Coding Standards issues live.

This problem is detected with phpcs which is run before the test suites runs.

Closing as outdated. See Automated testing. →

@uberhacker, Thank you for the idea!

After reading the documentation for \Drupal\Core\Updater\Module::getInstallDirectory() I think this change should not be made. And the proposal doesn't met the Criteria for evaluating proposed changes → . In this case, there is not demonstrated demand and support for the change. Therefore closing as won't fix.

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

The two changes in the patch have been fixed elsewhere

• update.theme_install was removed in 📌 Remove adding an extension via a URL Postponed
• update.theme_update was changed to what is in the patch 🐛 Broken Breadcrumb on Appearance tab. Fixed

This is now outdated.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

@longwave, thanks for identifying what issue fixed this problem.

I do think that the test case added here should be added. The test fixture added here is different from the existing ones in that it has a Major+2 version with alpha and beta releases. I am setting to Active to self review and add comments.

On 11.x, I changed the Drupal version to 9.5.11 and checked for updates. Here are the results.

The new test uses a release history with only the supported releases, 10.2.0, 10.2.2, 11.0.0-alpha1, and 11.0.0-beta1. And runs with an installed release of 9.x.y, And a sample output from the test.

Those indicate this is fixed, although I don't know when.

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

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies. Also, restoring the template.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

It is my understanding the the 'Needs Review Queue Initiative' tag is added after the issue has been reviewed by that initiative, so they can monitor the activity on reviewed issue. The definition → of this special tag supports my thinking. Am I wrong?

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

So, I came to two different conclusions based on my git research. That doesn't help this at all!

I have updated the issue summary

@avpaderno, do you have a suggestions to fix this?

Migrate Drupal UI is scheduled for deprecation and removal. Let's put our efforts elsewhere

📌 [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL Fixed

Migrate Drupal UI is scheduled for deprecation and removal. Let's put our efforts elsewhere

📌 [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL Fixed

The plan has changed, see 📌 [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL Fixed

quietone → created an issue.

Migrate Drupal and Migrate Drupal Ui are to be removed from core without replacement, this should not be needed.

D10 was released in Dec 2022, closing this.

Migrate drupal and Migrate drupal ui are going to be removed from core without replacement.

This now has all the sign off required. Therefor, setting to fixed.

The release managers discussed this at the end of May and I am only now commenting. We noted that this is part of the Workflow Initiative but that initiative has ended and there isn't anything here for us to review. We also are aware that there is the Trash module and a core issue to move that to core, 📌 Add experimental Trash module Active .

This issue was also closed; as won't fix in 2017 and then re-opened in 2019. But since then there has been no activity here despite a prompt for more information 2 years ago. After 5 years without comment here from someone from the workflow initiative it seems sensible to restore the won't fix from 2017.

And, if that there is a desire to add this to core, then it would be better to start fresh with a new initiative or plan. Therefore, I am going to close this. I trust someone will correct that if it is wrong.

No longer postponed on Workspaces

Add Updating a published change record