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

Improve prePR and CI feedback, esp. for new contributors #204

Closed
armanbilge opened this issue Mar 9, 2022 · 2 comments · Fixed by #542
Closed

Improve prePR and CI feedback, esp. for new contributors #204

armanbilge opened this issue Mar 9, 2022 · 2 comments · Fixed by #542

Comments

@armanbilge
Copy link
Member

After an extensive discussion on Discord about "What prePR means to you", I think we roughly came to this consensus:

  1. prePR should only do polish tasks, which are essentially things that can be fixed by the task itself. Additionally, the polish check should be a separate job in CI, like suggested in Project verification job #93.
  2. We should try and relax the CI-requires-approval-for-new-contributors, since it really kills the feedback loop. I'm pretty sure there's a mode to only require it for new GH users, and I bet an admin can set this for the whole org.

Roughly, prePR/polish tasks should meet this criteria:

  • it happens without any configuration or control
  • it's infallible when it runs
  • once you run it locally, the check in ci will never fail
  • things that check something and fix it if needed on their own

As much as CI failures are annoying, at least maintainers can help. We don't want prePR to discourage contributors from creating a PR in the first place by failing with e.g. mima issues that they don't understand or know how to fix.

There's also a lot of interest in an idea to ask CI to run prePR for you like in #69, but it's possibly tricky to implement.

@armanbilge
Copy link
Member Author

See also #203 (comment).

@rossabaker
Copy link
Member

Note that some scalafix are "polish" (e.g., organize imports), and some require intervention (e.g., no fs2.Compiler[Sync]). And some of those that are "polish" are semantic rules, and therefore relatively slower.

@armanbilge armanbilge added this to the v0.5.0 milestone Jun 1, 2022
@armanbilge armanbilge removed this from the v0.5.0 milestone Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants