search

rust-lang / highfive

Github hooks to provide an encouraging atmosphere for new contributors
Apache License 2.0
185 stars 128 forks source link

Add new reviewer to librustdoc static files #367

Closed Folyd closed 2 years ago

Folyd commented 2 years ago

Ping @Folyd if some changes occurred in librustdoc's HTML/CSS/JS.

Related context: https://github.com/rust-lang/rust/pull/91062#issuecomment-987516134

Mark-Simulacrum commented 2 years ago

I am not sure we want to add external folks to the ping lists here -- it implies a level of officialness, and for example the target tier policy talks about not pinging and such for relatively similar reasons.

So my initial inclination is to suggest that this is likely not quite the right path to doing this, even though it looks like a trivial change. It might be useful to have a discussion amongst the rustdoc team as to whether this is the expected way of dealing with changes here -- I recall that other parts of rustdoc, like the JSON API crate, are explicitly versioned (which may imply an opportunity for changelogs / blog posts / something on bumps).

jyn514 commented 2 years ago

I really don't want any part of the CSS to be a stable API. The alternative to this is for @Folyd to keep only finding breaking changes after they occur; I think it's reasonable to ping them ahead of time without implying that the PR needs to change. My understanding is that the target policy is saying team members shouldn't be pinged for breakage to tier 3 targets; I don't think it's unreasonable for the target maintainers to be pinged.

Mark-Simulacrum commented 2 years ago

I'm okay adding this since we're already putting in the comments (i.e., it's not a new comment that gets generated), so the added noise is minimal, but I think we may want to reconsider if the list grows.