Add way to do mentions of content changes #2171
Conversation
| None => write!(result, "Some changes occurred in {to_mention}").unwrap(), | ||
| None => match type_ { | ||
| MentionsEntryType::Filename => { | ||
| write!(result, "Some changes occurred in {entry}").unwrap() |
There was a problem hiding this comment.
Hmm, we don't actually use the relevant file paths for the filename. But I guess it doesn't hurt to collect them.
|
Amazing, thanks :) When can we start using this in rust-lang/rust? |
|
We don't have a staging environment, so starting from when it was merged. |
| fn patch_contains(patch: &str, needle: &str) -> bool { | ||
| for line in patch.lines() { | ||
| if (!line.starts_with("+++") && line.starts_with('+')) | ||
| || (!line.starts_with("---") && line.starts_with('-')) |
There was a problem hiding this comment.
I don't think we should trigger a mention when the text is removed.
There was a problem hiding this comment.
We discussed that above in #2171 (comment) and considered that removal might also be important. We can change that if you prefer since you're the only user.
There was a problem hiding this comment.
I don't know when you'd care about deletions. Additions is what I proposed originally. At least for the attributes I have in mind for this, removing them requires no extra care (beyond the things CI already ensures).
There was a problem hiding this comment.
Won't changes also count as a removal + addition?
There was a problem hiding this comment.
Yeah so they would still cause a ping even if we just scan + lines, which seems like reasonable behavior to me.
There was a problem hiding this comment.
Opened #2176 to change the behavior.
4 participants
This PR adds a way to do mentions of content changes (instead of just filename changes).
Fixes #1851