Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add contact github username for ecosystem checks #10260

Closed
ThiefMaster opened this issue Mar 6, 2024 · 7 comments
Closed

Add contact github username for ecosystem checks #10260

ThiefMaster opened this issue Mar 6, 2024 · 7 comments
Labels
ci Related to internal CI tooling

Comments

@ThiefMaster
Copy link
Contributor

My repo is one of the repos listed in the ecosystem checks. I would like to get automatically pinged on any PR that introduces changes resulting in new violations when linting my repo.

Context: I just upgraded ruff and got a bunch of violations due to #10238.

  1. I believe there is behavior hat may be considered a bug (see UP032 complains about format args performing function calls #10258) which I would have pointed out had I been pinged
  2. I think this change made the rule less useful (see UP032 became less useful in v0.3.1 #10259),

I would have liked to provide this feedback on the PR, before it got merged and shipped in a release.

@zanieb
Copy link
Member

zanieb commented Mar 6, 2024

This seems challenging to do well, but the sentiment is fair. The ecosystem checks are really for the person making changes though, they're not generally intended to be helpful to the repositories we target in them. Ideally preview mode would be providing the avenue for this kind of feedback.

@ThiefMaster
Copy link
Contributor Author

The ecosystem checks are really for the person making changes thoug

Sure, but why not let people whose repos are part of the ecosystem checks get 1) a heads-up about changes affecting their repos and 2) give early feedback to the contributor of such a PR.

Preview mode doesn't really help because 1) I either need to have a CI that uses bleeding-edge ruff (from master) that runs frequently enough or 2) I need to wait for a release and then discover breakage and then hope it gets fixed in the next release (and I'd skip the "bad" release)

This seems challenging to do well

Couldn't it just add the name in the this line of the ecoystem check result comment if there's a change in violation count and contact info is available:

indico/indico (+39 -0 violations, +0 -0 fixes) [cc @ThiefMaster]

@MichaReiser
Copy link
Member

@ThiefMaster I'm sorry for the disruption that this change has caused. In hindsight, this should have been a preview style change that would have allowed us to gather more feedback.

Couldn't it just add the name in the this line of the ecoystem check result comment if there's a change in violation count and contact info is available

That could work based on opt-in by maintainers. Would you be interested PRing said change to the ecosystem checks?

@MichaReiser MichaReiser added help wanted Contributions especially welcome ci Related to internal CI tooling labels Mar 7, 2024
@zanieb
Copy link
Member

zanieb commented Mar 11, 2024

I'm still kind of hesitant to support this. We'll discuss it further internally but I would not recommend starting work on it.

@zanieb zanieb added needs-decision Awaiting a decision from a maintainer and removed help wanted Contributions especially welcome labels Mar 11, 2024
@ThiefMaster
Copy link
Contributor Author

In any case, I wouldn't mind making a PR. But only if it'll get merged of course, not a fan of wasting my time. :)

@zanieb
Copy link
Member

zanieb commented Mar 11, 2024

I'm sorry I just don't feel comfortable committing to providing this. We do our best to assess changes and reduce breakage for downstream projects andI want to encourage feedback and interaction but I'd prefer another way to do it.

We could look into something outside of the ruff repository that e.g. nightly reports changes from ruff's main branch with preview enabled. I think something in that format makes more sense than incorporating this into pull requests which can be relatively noisy.

@charliermarsh
Copy link
Member

I'm going to close this for now -- I think the decision above is not to support it, but open to other ideas on how we can improve ecosystem notifications.

@charliermarsh charliermarsh closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
@charliermarsh charliermarsh removed the needs-decision Awaiting a decision from a maintainer label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to internal CI tooling
Projects
None yet
Development

No branches or pull requests

4 participants