Add support for shared organization policies #321
Conversation
| return errors.WithMessage(err, fmt.Sprintf("Failed to fetch configuration: %s", fetchedConfig)) | ||
| } | ||
|
|
||
| fetchedConfig := b.ConfigFetcher.ConfigForPR(ctx, prctx, client) |
There was a problem hiding this comment.
just checking: we don't return an error anymore here because if one exists, its embedded in the fetchedConfig and is then evaluated below in ValidateFetchedConfig with better errors?
There was a problem hiding this comment.
Yes, previously, loading errors were handled by failing and not posting anything on the pull request. Only parsing errors resulted in a status check. In the new model, both types of errors are handled the same way.
|
|
||
| return nil, nil | ||
| } | ||
| b.PostStatus(ctx, prctx, client, "error", msg) |
There was a problem hiding this comment.
nit (since it seems like PostStatus is showing up in this PR): should we have something like PostErrorStatus or a constant so that callers don't need to pass around magic "error" strings?
There was a problem hiding this comment.
This is an existing function that is called in slightly different places now. It's also called with "pending" and "success" (so the possible check statuses in GitHub). We could make defined functions for each status, but I think I'd leave that for a separate PR.
| // does not exist or is invalid, the returned error is nil and the appropriate | ||
| // fields are set on the FetchedConfig. | ||
| func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) (FetchedConfig, error) { | ||
| func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) FetchedConfig { |
Refactor configuration loading to use the go-githubapp/appconfig package. This mostly adds support for shared organization policies (in the .github repository by default), but should also make error messages more accurate. It also changes the policy link in the details view to point to the resolved policy, instead of the local policy file.
bluekeyes
force-pushed
the
bkeyes/appconfig
branch
from
3e5c609 to
a3ec920
Compare
Refactor configuration loading to use the
go-githubapp/appconfigpackage. This mostly adds support for shared organization policies (in the.githubrepository by default), but should also make error messages more accurate.It also changes the policy link in the details view to point to the resolved policy, instead of the local policy file.
I'll need to rebase this after #320 merges, but I didn't want to combine the
go-githubupgrade with the functionality changes. Also needs palantir/go-githubapp#94 to merge to maintain current behavior with empty configuration files.