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 behaviour when .scala-steward.conf repo config is invalid (ConfigIsInvalid) #3098

Open
rtyley opened this issue Jun 22, 2023 · 1 comment

Comments

@rtyley
Copy link
Contributor

rtyley commented Jun 22, 2023

At the moment, when Scala Steward encounters a repo with an invalid .scala-steward.conf, this only has a relatively mild effect:

  • "Note that the Scala Steward config file .scala-steward.conf wasn't parsed correctly" is displayed (with helpful details) near the bottom of the (potentially very long) PR description:
image

Although the message is very helpful, it does run the risk of being missed - in our case, we had a situation where a a small typo led to a configuration failure, the entire config being ignored, and the affected team missing the helpful warning, and being very puzzled by unwanted behaviour from Scala Steward that they thought they'd configured against.

Possible improvements

Most preferred first - if Scala Steward encounters an invalid config file for a repo, it should:

  • Abort processing that repo - otherwise it risks taking actions and raising PRs that users have specifically tried to avoid - it is better to do nothing. A downside is that feedback direct to the repo is lost, but even in the absence of improved feedback there, I feel that not performing a nurture is the right thing to do. Thanks to Add GitHub Action Job Summary summarising failing repos #3071, the failure for that repo will at least be clearly visible at a job level.
    • Subsequent improvements might look at other ways of surfacing the failure information against a repo:
      • raise a repo issue, rather than a PR, detailing the problem, if the forge type supports it
      • a status badge for a repo's README that shows how long since Scala Steward successfully ran against the repo
  • If not abort, then place the warning much more prominently, ie at the top of the PR description, above the long list of updates.

Like I say, I'd prefer to simply abort processing, but I'm happy enough to raise a PR against either?

@fthomas
Copy link
Member

fthomas commented Dec 8, 2023

  • Abort processing that repo

We could do that if everybody who receives Scala Steward's PRs would have access to the run logs. But that is not the case for the public instance for example. If we just abort processing, people wouldn't know (and couldn't find out) why they don't receive PRs anymore.

Raising an issue if the config is invalid and then abort processing would be nice but would be much more work than placing the warning more prominently.

  • place the warning much more prominently

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants