Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Add 26 Missing GitLab Settings #1987

Merged
merged 4 commits into from
Aug 20, 2024
Merged

Add 26 Missing GitLab Settings #1987

merged 4 commits into from
Aug 20, 2024

Conversation

Jitsusama
Copy link
Contributor

@Jitsusama Jitsusama commented Aug 14, 2024

I have discovered ~ 60 settings that have been added to GitLab's API but are currently missing in this project. In this PR I'm taking the next stab at correcting this by adding in 26 missing settings, leaving 27 left.

These settings were found by perusing documentation and code in response to gitlab-org/terraform-provider-gitlab#6332.

This required adding an additional type as it takes an object with keys
of varying types, so I couldn't follow the standard pattern of
specifying it as a string map with some standard value type.
@Jitsusama Jitsusama changed the title Add 25 Missing GitLab Settings Add 26 Missing GitLab Settings Aug 14, 2024
Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but one comment related to the additional struct.

types.go Outdated
AllowForcePush bool `json:"allow_force_push,omitempty"`
AllowedToMerge []int `json:"allowed_to_merge,omitempty"`
DeveloperCanInitialPush bool `json:"developer_can_initial_push,omitempty"`
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to have this struct defined below the Settings struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be addressed now. Let me know if you would like it moved somewhere differently, I assumed you meant the bottom of the file, but I just recognized that you might have meant right below. I don't mind moving it to wherever you'd prefer.

settings.go Outdated
DefaultArtifactsExpireIn *string `url:"default_artifacts_expire_in,omitempty" json:"default_artifacts_expire_in,omitempty"`
DefaultBranchName *string `url:"default_branch_name,omitempty" json:"default_branch_name,omitempty"`
DefaultBranchProtection *int `url:"default_branch_protection,omitempty" json:"default_branch_protection,omitempty"`
DefaultBranchProtectionDefaults *BranchProtectionDefaults `url:"default_branch_protection_defaults,omitempty" json:"default_branch_protection_defaults,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field should be set to a BranchProtectionDefaultsOptions struct which contains the required fields as pointers (see other option structs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be addressed now.

@Jitsusama Jitsusama requested a review from svanharmelen August 15, 2024 14:19
@Jitsusama
Copy link
Contributor Author

@svanharmelen; mind taking another look at this, I think I have addressed all of your concerns and would love to cut another MR soon with the last batch of changes.

@timofurrer
Copy link
Contributor

@Jitsusama while you are at it, do you mind adding receptive_cluster_agents_enabled that was recently added with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162514 ?

@Jitsusama
Copy link
Contributor Author

@Jitsusama while you are at it, do you mind adding receptive_cluster_agents_enabled that was recently added with https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162514 ?

I'll add it to the next batch that covers the Rs @timofurrer!

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the ordering (minor tweak), but looks good now 👍🏻 Thanks!

@svanharmelen svanharmelen merged commit e57ef95 into xanzy:main Aug 20, 2024
3 checks passed
@Jitsusama Jitsusama deleted the Settings-Parameter-Sync-Batch-2 branch August 22, 2024 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants