dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.63k stars 993 forks source link

Don't @mention folks in dependabot PR descriptions #2820

Closed matklad closed 3 years ago

matklad commented 3 years ago

Hi!

I've just got pinged by a dependabot PR to a repository, which is unrelated to my work: https://github.com/rcos/Telescope/pull/31#event-4071547309

The reason for that is dependabot includes release notes for the libary I actually contributed to, and release notes included @matklad string.

image

https://github.com/serde-rs/json/releases/tag/v1.0.60

Such mentions should be escaped to not punish library developers with unrelaed GH notifications :-)

jurre commented 3 years ago

Hi @matklad, yes we indeed have some code in place to escape mentions like this so this is a bug. Will look into what's going wrong here, thanks for reporting it!

jurre commented 3 years ago

Hmm, I'm actually surprised you got a notification, the raw markup for that pull request is:

Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.59 to 1.0.60.\n<details>\n<summary>Release notes</summary>\n<p><em>Sourced from <a href=\"https://github.com/serde-rs/json/releases\">serde_json's releases</a>.Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.59 to 1.0.60.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/serde-rs/json/releases">serde_json's releases</a>.</em></p>
<blockquote>
<h2>v1.0.60</h2>
<ul>
<li>Add <code>impl FromIterator&lt;(impl Into&lt;String&gt;, impl Into&lt;Value&gt;)&gt; for Value</code>, which collects a Value::Object (<a href="https://github-redirect.dependabot.com/serde-rs/json/issues/733">#733</a>, thanks <a href="https://github.com/matklad">@matklad</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/serde-rs/json/commit/6a4cd8d21879274f07baa3d35f6e5b2d43057986"><code>6a4cd8d</code></a> Release 1.0.60</li>
<li><a href="https://github.com/serde-rs/json/commit/b9598ce50f3376eb48865711168757065bcd9a39"><code>b9598ce</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/serde-rs/json/issues/733">#733</a> from matklad/from-iter-of-pairs</li>
<li><a href="https://github.com/serde-rs/json/commit/0c4b4dfc8ae8e6d32d3721567b6f26cc2e49cd5f"><code>0c4b4df</code></a> Allow collecting an iterator of pairs into JSON object</li>
<li><a href="https://github.com/serde-rs/json/commit/efc910404a6c8863188e733a0809b594b52d11b4"><code>efc9104</code></a> Suppress clippy unnecessary_wraps lints</li>
<li><a href="https://github.com/serde-rs/json/commit/bda64205e3abd2788d0e4006ceb94c7236da7c56"><code>bda6420</code></a> Resolve clippy comparison_to_empty lint</li>
<li><a href="https://github.com/serde-rs/json/commit/ec7eeb6933670655b28862fe1571e17801cad788"><code>ec7eeb6</code></a> Suppress new manual_range_contains lint</li>
<li>See full diff in <a href="https://github.com/serde-rs/json/compare/v1.0.59...v1.0.60">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.59&new-version=1.0.60)](https://docs.github.com/en/github/managing-security-vulnerabilities/configuring-github-dependabot-security-updates)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>

The relevant bit is thanks <a href="https://github.com/matklad">@matklad</a>), that's explicitly escaped to not use @matklad.

I'm going to check with the notifications team how you ended up with a notification for this pull request.

jurre commented 3 years ago

Testing if @evil-jurre results in a mention from a comment

jurre commented 3 years ago

Ah, ok so what actually happened here is that you got mentioned in the email reply from @Alfriadox.

That reply contains a stripped down version of the PR body, and that in turn becomes a mention:

    - Add impl FromIterator<(impl Into<String>, impl Into<Value>)> for
    Value, which collects a Value::Object (#733
    <https://github-redirect.dependabot.com/serde-rs/json/issues/733>,
    thanks @matklad <https://github.com/matklad>)

I wonder if there's something we can do specific to Dependabot, maybe escaping the username with backticks will prevent a mention in such replies, but maybe there's also something we can do more generally for @mentions in email replies GitHub wide. 🤔

feelepxyz commented 3 years ago

I wonder if there's something we can do specific to Dependabot

Oh I have a vague memory of us injecting a zero-width whitespace character between @ and username but we got rid of this in a refactor, sounds like something like that would work here?

Less sure about changing behaviour for all email replies as it seems like a useful feature for some.

jurre commented 3 years ago

I wonder if there's something we can do specific to Dependabot

Oh I have a vague memory of us injecting a zero-width whitespace character between @ and username but we got rid of this in a refactor, sounds like something like that would work here?

Less sure about changing behaviour for all email replies as it seems like a useful feature for some.

Hehe, yes that would probably work! We could also see if we could do something like, this was suggestion by the notifications folks:

<a href="https://github.com/evil-jurre"><code>@evil-jurre</code></a>

I think it looks OK: code>@evil-jurre</code and should not cause a mention, I'm just not entirely sure how that would render in an email reply, but assuming that a <code> block will not result in a mention in that case.