-
Notifications
You must be signed in to change notification settings - Fork 871
fix: Add missed parameter to the github_organization_ruleset #2545
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
fix: Add missed parameter to the github_organization_ruleset #2545
Conversation
|
It would be nice to have this merged. Can someone review it? |
|
@kfcampbell Can we please get this fix merged and released? |
|
Hi @kfcampbell can you please review and merge this? |
|
@yurii-kysel Should probably also update https://github.com/integrations/terraform-provider-github/blob/main/website/docs/r/organization_ruleset.html.markdown in this PR, too |
|
fairly sure this PR fixes #2597 - would be great to have it approved/merged :) |
|
oh and #2542 would get closed out too (already linked) |
| Optional: true, | ||
| Description: "Whether pull requests targeting a matching branch must be tested with the latest code. This setting will not take effect unless at least one status check is enabled. Defaults to `false`.", | ||
| }, | ||
| "do_not_enforce_on_create": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually also need it on the required_workflows struct too
| "required_workflows": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the error you could prevent future panics in
terraform-provider-github/github/respository_rules_utils.go
Lines 343 to 348 in 9fceeda
| doNotEnforceOnCreate := requiredStatusMap["do_not_enforce_on_create"].(bool) | |
| params := &github.RequiredStatusChecksRuleParameters{ | |
| RequiredStatusChecks: requiredStatusChecks, | |
| StrictRequiredStatusChecksPolicy: requiredStatusMap["strict_required_status_checks_policy"].(bool), | |
| DoNotEnforceOnCreate: &doNotEnforceOnCreate, | |
| } |
params := &github.RequiredStatusChecksRuleParameters{
RequiredStatusChecks: requiredStatusChecks,
}
if v, ok := requiredStatusMap["do_not_enforce_on_create"].(bool); ok {
params.DoNotEnforceOnCreate = &v
}
if v, ok := requiredStatusMap["strict_required_status_checks_policy"].(bool); ok {
params.StrictRequiredStatusChecksPolicy = v
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(but the Default: false should prevent that from happening 😅 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the status on this?
|
None of that matters unless M$ Github actually allocates time for their people to merge PRs. Plenty of us are actual paying GHE customers and are being ignored. |
Resolves #2542
Before the change?
github_organization_rulesetthat cause an error, as it's required inrespository_rules_utils.expandRulesAfter the change?
github_organization_rulesetwith the default valuePull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!