Support notification on error

[tunnckoCore]

Seems like my repo doesn't get updated because the #125 where I use color: #333333 (with the hex), so it errors (probably? the API?)

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[tunnckoCore]

Yup.

[gr2m]

can you try color: "#333333"? color: #333333 is valid YML, but everything after # will be ignored as per specification

[travi]

it's worth noting that the api docs for labels specifically mention leaving the leading # off of the color. short of special handling in this application, along the lines of #125, i would only expect this to work without the #. also, i do have configs that define colors without the # and they are being set correctly using that approach.

[tunnckoCore]

Yes, okay, I'm not stupid.

The thing isn't what can be done or that I didn't read the docs. The things is to somehow report such errors so user can know what and why is happening.

Anyway, we will close that cuz you all want.

[travi]

The thing isn't what can be done or that I didn't read the docs. The things is to somehow report such errors so user can know what and why is happening.

absolutely. sorry if my comment was unclear. i definitely think it would be valuable to find a way to report errors of all kinds rather than failing silently. i'm going to keep this issue open to track that need.

my comment was to clarify the requested test. based on the docs, i was simply pointing out that i don't think the quoting is the problem. i think the api actually would fail in this case. regardless, it would be valuable to provide feedback about the failure.

[kyleroush]

The checks API or the status API might be a good way to resolve letting people know if the settings were updated or not. I think the checks would be the better way as you can provide more information about what failed.

here is a diff where I add it https://github.com/probot/settings/compare/master...kyleroush:master With this if the settings are all updated it will add a successful check or if the update fails it can describe what failed. Right now when it is failing I am just showing the response body of what failed which is not the prettiest. Here is an example of it failing https://github.com/kyleroush/test4/runs/253938319

But doing something like this would probably involve having to request new authorization persions to the checks API for the bot

[gr2m]

I did a similar thing for the WIP app, I use it to explain why a pull request is work in progress. You'd have to add a check in case of a forbidden error, as people are slow to accept new permissions, and you want to handle that case gracefully.

[kyleroush]

The main question I have for adding this check would be

1. what message would we want to display on success?
2. what message would we want to display on failure?
3. If they push a bunch of commits do we put the check on the last commit or the last commit that modified the settings files?
4. what if multiple API request fail? Do you fail fast and just report the first failure? Or do you try and gather all the errors?

for the future

1. in the future would the rerun button on the check be enabled so that people can ensure the settings are up to date and wipe out any manual changes?

[gundalow]

There is another bot (I'll need to find it again) which has a webservice where you can point it at a config file (github.com URL)and it validates it. That might not catch all the problems, though it might help some cases of invalid config.

[travi]

i do think linting/validating of a config file could be really useful, but i'm not sure how to do so cheaply with a small team of volunteer maintainers. defining a schema or similar to validate against is the main work that we would need to find time to complete, but i'm open to discussing further, especially if there are ideas around tools that could help simplify.

this does seem separate from providing feedback as a result of errors, though. would you mind opening a separate issue for conversation around linting/validating of config files? if you do find the tool you're thinking of, i think it would be a valuable detail to include there.

[travi]

defining a schema or similar to validate against is the main work that we would need to find time to complete, but i'm open to discussing further, especially if there are ideas around tools that could help simplify.

as a quick follow up, i've seen that it is possible to validate yaml files against a json-schema, so i've opened https://github.com/probot/settings/issues/182 for looking more into that. i would be more than open to someone investigating that and helping it move forward

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[Luttik]

I think this is pretty essential. As far as I know, this app just fails silently without any way whatsoever to find out what went wrong. Especially for an app that has so much access to organisations it is essential to have good messaging to enable trust.

[phnx47]

I just installed app and nothing happen. I don't know what I need to fix in config. Notification is very important.

[sellout]

I’m trying to work around this issue currently. Since the app makes a request per section of the config, I figured I could narrow down where some error in my config is to at least a single “section” by knowing which order the sections are updated and seeing which was the last to be successfully changed.

Looking at https://github.com/repository-settings/app/blob/edc40ce00012589c413270710e7e7d1510aa470c/lib/settings.js#L12-L24

the branch settings are clearly applied last, but is there an expectation as to the order of the entries in ...rest? Through a few levels of indirection, I came across this statement: “The traversal order, as of modern ECMAScript specification, is well-defined and consistent across implementations. Within each component of the prototype chain, all non-negative integer keys (those that can be array indices) will be traversed first in ascending order by value, then other string keys in ascending chronological order of property creation.”

In the case of reading a YAML file, does the property creation order match the order they are in the file? E.g., in my file, the sections (in order) are branches, labels, and repository. Can I expect the order of application to be labels, repository, then branches (since branches is explicitly applied last in the code)? And so, if the labels weren’t updated, then I know the error is in that section, because that should be the first update sent?

[sellout]

Working under the theory that the order is what I was expecting, I debugged my labels (one of the descriptions was too long) and now the labels have been updated, but repository (and branches) have not been. So I think my theory is right 🤞🏼 and now I can go on to figure out what’s incorrect in my repository settings.