[MAINT][FEAT] Fix Org Ruleset tests && enable allowed_merge_methods#2976
[MAINT][FEAT] Fix Org Ruleset tests && enable allowed_merge_methods#2976nickfloyd merged 20 commits intointegrations:mainfrom
allowed_merge_methods#2976Conversation
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
@stevehipwell Sure! Would you want to rebase that branch on top of go-github-v68, so that I could use it as a base for this PR? |
|
@deiga the acceptance tests PR should be compatible with the v6 releases, so the v7 branch would be rebased on it. I'm currently re-running the tests to get it ready to be merged and I'm picking up defects in the currently released code. |
3752c16 to
d222f43
Compare
stevehipwell
left a comment
There was a problem hiding this comment.
I've added some minor suggestions on structuring the TF test code to make it easier to read and more idiomatic. But my main concern here is the actor ID change.
Also is there a reason you didn't add the other missing ruleset rules? Just curious.
allowed_merge_methods
…sport` Updated the logging transport in the RateLimitedHTTPClient function to use NewLoggingHTTPTransport. As the subsystem would need to be explicitly inititated in each resource Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
It's a required field in the API and go-github doesn't use `omitempty`, so it submits `nil` if it isn't sent explicitly. This change tries to keep it in the state, without having a configuration option for it (poor choice?) And it defaults to all 3 available merge methods if it can't set something from the state. Signed-off-by: Timo Sand <timo.sand@f-secure.com>
This is a required field and the SDK doesn't omit if it's empty Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
…on_rulesets_without_errors` to pass Signed-off-by: Timo Sand <timo.sand@f-secure.com>
…null` Signed-off-by: Timo Sand <timo.sand@f-secure.com>
…_all_bypass_modes` Main fix: `bypass_actors` is returned as sorted from GH API so tests need re-indexing Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>
Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>
Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
With the recent changes to the SDK and the Create method we less often purely refresh state if it hasn't changed upstream, leading to us ignoring the return value of `actor_id` often Signed-off-by: Timo Sand <timo.sand@f-secure.com>
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
62ed5c4 to
ac1b510
Compare
stevehipwell
left a comment
There was a problem hiding this comment.
This looks great.
LGTM
Resolves #2530, #2536, #2717
Before the change?
TEST="./github" TESTARGS="-run TestGithubOrganizationRulesets" make testaccwould fail due to multiple issues in the test setupAfter the change?
TEST="./github" TESTARGS="-run TestGithubOrganizationRulesets" make testaccwill show all tests as passingNewSubsystemLoggingHTTPTransporttoNewLoggingHTTPTransportas we don't actually initialize a Subsystem anywhere, which creates spam.github/resource_github_organization_ruleset.goandgithub/resource_github_repository.goto use Context aware CRUD methodsrules.pull_requests.allowed_merge_methodshandling togithub/resource_github_repository_ruleset.goandgithub/resource_github_organization_ruleset.goPull request checklist
Schema migrations have been created if needed (example)Does this introduce a breaking change?
Please see our docs on breaking changes to help!