Demote routine log events from "notice" to "info"

As I see media and user using notice
- \Drupal\media\MediaForm::save()
- \Drupal\media\MediaTypeForm::save()
- \Drupal\user\Form\UserCancelForm::submitForm()
- \Drupal\user\RegisterForm::save()

probably there's more and I bet it needs change record

Thank you, makes sense. I find it ready

Thanks for continuing to improve the log levels!

I'm triaging RTBC issues.

I read the IS and comments and was wondering if there was a Meta or some other discussion where these changes were agreed to. I found #3255699: Demote routine content entity log entries from "notice" to "info" and read the IS and comments over there. In particular is comment #17 where this task for all of core was scoped. And it turns out that that the patch over there was not modified in response. The same is true for this issue, it is using a different scope than recommended by a core committer.

There may be reasons for that change, and if there are, they should be commented to allow discussion. I also recommend making a Meta from comment #17so that it easier to follow the changes and to ensure that all the messages are changed. It would also allow others to pick up a piece of this task.

I read that xjm recommends only one change record. That is a good idea but only when the commits are to the same branch. Also, when there is a new CR, like in this issue, it should have a different name than the previous one to avoid confusion. I would go so far as to say the CR from the other issue should have a title change to reflect which kinds of messages are demoted. But I haven't checked to see if that would make the title too long.

Because of the scoping I am setting this back to NW.

@mfb, I am referring to the scoping set out by xjm in comment 17 of the current parent issue, #3255699-17: Demote routine content entity log entries from "notice" to "info".

Split from main issue looks good

Seems unrelated.

Seems random again

This looks great. However, instead of creating a new CR, can we just attach https://www.drupal.org/node/3381101 and update it to include the changes from this issue? Thanks!

@xjm may be late but I don't follow, the attached link is to this CR. So not sure whats meant by

can we just attach https://www.drupal.org/node/3381101 and update it to include the changes from this issu

lol my bad. Don't mix sleep medication and the RTBC queue, folks.

Oh that said I would like other issues to be added to the scope. All the "Demote notice..." issues should be merged into one unless there are like 50+ such reductions.

Thanks @mfb for closing those others out.

Ahhh I see what happened. The key phrase from #3255699-17: Demote routine content entity log entries from "notice" to "info" is:

If we need to split it up

In this case, its size and change pattern did not merit being split up. It's truly impressive that I managed to write a 3000-word comment and still did not manage to get the key point across. 😅 It is explained in the scope doc I linked, but in reality it's a lot of info to process.

The key considerations are the size of the changeset in added plus removed lines, and whether the changes can be scanned and reviewed without context. If you need to look beyond the changed lines to get the context, smaller scopes are better. For example, adding return typehints or documentation requires reading an entire method as well as looking at all its callers and parent or sibling or child methods, so small scopes are best for that.

If something is scannable, with each change following an identical pattern, and only requires a git diff --color-words to validate, and the total scope of potential changes is already documented along the lines of my comment from the other issue, it's safe to have at least 100-200 total lines to review (50 to 100 one-line changes).

The only times we would split up a "no context required color-words issue" would be:

• If it were mixing different patterns of otherwise scannable changes
• If it were more than 300-400 lines (150-200 one-line changes)
• If parts were patch-eligible and other parts were minor-only
• f there were interdependencies with other issues
• If there were merge conflicts with prioritized issues.

Edit: reformatted for readability.

If you want to learn more about why these details of scoping are important, the video of my DDD Vienna session on code review explains the psychology behind all this. 🙂

Ohh, on the CR front, I meant adding them to the previous CR in https://www.drupal.org/node/3259469/edit. I updated that CR to include this issue and its changes, and deleted the new draft.

Crediting @mfb for patch work, @smustgrave for helping re: the CRs, and @quietone and @andypost for asking scope questions.

Is there a separate followup for the media events @andypost suggested? If exists and is RTBC-ish, it can also be included here. If not, we should file it. Thanks!

Promoting to normal; "minor" should only be used for nitpicks. This change improves the UX of watchdog, so it's not a nothing-change.

Oh, and NW until the followup is either filed or merged into this issue.

Removing follow up tag.

OK, sounds good!

Committed to 11.x and cherry-picked to 10.2.x. Since this is a behavior change for things in the admin UI, I was on the fence about backporting it to a patch release. I checked the previous issue and it was not backported past 10.1.x, so I followed the same pattern here.

Thanks everyone!

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)