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

Added change record draft and release notes snippet. @andypost I'm not proposing further changes to user and media logging at this time, because we already updated those modules to log routine events as "info" and more significant events as "notice". Certainly if there is consensus though, more log level updates can be added to this issue.

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 📌 Demote routine content entity log entries from "notice" to "info" Fixed and read the IS and comments over there. In particular is comment #17 📌 Demote routine content entity log entries from "notice" to "info" Fixed 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 #17 📌 Demote routine content entity log entries from "notice" to "info" Fixed so 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.

@quietone I'm unclear what scope you are proposing. For example, we could split this issue into 5 issues: One for email action, one for comment posting and deleting, one for contact emails, one for search, and one for taxonomy create and update. Maybe that's too much issue overhead, or does that sound reasonable?

@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" → .

@quietone ok, that doesn't mention taxonomy so I kinda have to extrapolate. How about splitting this into 3 issues: one for email action and contact emails, one for comment post/delete and taxonomy create/update (since those are both content entities), and one for search? My take would be that this is not worth splitting into multiple issues, but *shrug* whatever's easier.

Split off two child issues: 📌 Demote email log events from "notice" to "info" Closed: duplicate and 📌 Demote search log events from "notice" to "info" Closed: duplicate and rerolled with just the content entity related hunks.

Split from main issue looks good

The last submitted patch, 9: 3381078.patch, failed testing. View results →

Seems unrelated.

The last submitted patch, 9: 3381078.patch, failed testing. View results →

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.

Lol ok restoring the initial patch, issue summary and title

Thanks @mfb for closing those others out.

Fixed typo in issue summary

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.

If something is scannable 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 time we would split up a "no context required color-words issue" would be if it were more than 300-400 lines (150-200 one-line changes), or if parts were patch-eligible and other parts were minor-only, or if there were interdependencies or merge conflicts with other issues.

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.

As I mentioned in #3 I am not proposing further changes to Media logging

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.)

Automatically closed - issue fixed for 2 weeks with no activity.