-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat!: Add enterprise security configurations, update API fields #3812
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
sorry forgot to add the generated files after my latest change.. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3812 +/- ##
==========================================
+ Coverage 92.27% 92.32% +0.05%
==========================================
Files 192 193 +1
Lines 13896 13988 +92
==========================================
+ Hits 12823 12915 +92
Misses 884 884
Partials 189 189 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
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.
please let me know if I should drop the commit with breaking changes and what you think about the other commits
I think that is fine, thanks. Please address the few minor suggestions I've made, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29
| // GitHub API docs: https://docs.github.com/rest/code-security/configurations#create-a-code-security-configuration-for-an-enterprise | ||
| // | ||
| //meta:operation POST /enterprises/{enterprise}/code-security/configurations | ||
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
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.
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { | |
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, config CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
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.
I think should be a separate struct (fields name and description are required):
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { | |
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, config CreateCodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
type CreateCodeSecurityConfiguration struct {
Name string `json:"name"`
Desription string `json:"description"`
// all other optional fields
}| // GitHub API docs: https://docs.github.com/rest/code-security/configurations#update-a-custom-code-security-configuration-for-an-enterprise | ||
| // | ||
| //meta:operation PATCH /enterprises/{enterprise}/code-security/configurations/{configuration_id} | ||
| func (s *EnterpriseService) UpdateCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
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.
| func (s *EnterpriseService) UpdateCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { | |
| func (s *EnterpriseService) UpdateCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, config CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
| } | ||
|
|
||
| // AttachCodeSecurityConfigurationToRepositories attaches an enterprise code security configuration to repositories. | ||
| // |
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.
| // | |
| // `scope` is the type of repositories to attach the configuration to. | |
| // Can be one of: `all`, `all_without_configurations`. | |
| // |
| } | ||
|
|
||
| // SetDefaultCodeSecurityConfiguration sets a code security configuration as a default for an enterprise. | ||
| // |
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.
| // | |
| // `defaultForNewRepos` specifies which types of repository this security configuration should be applied to by default. | |
| // Can be one of: `all`, `none`, `private_and_internal, public`. | |
| // |
| // GitHub API docs: https://docs.github.com/rest/code-security/configurations#set-a-code-security-configuration-as-a-default-for-an-enterprise | ||
| // | ||
| //meta:operation PUT /enterprises/{enterprise}/code-security/configurations/{configuration_id}/defaults | ||
| func (s *EnterpriseService) SetDefaultCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, newReposParam string) (*CodeSecurityConfigurationWithDefaultForNewRepos, *Response, error) { |
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.
| func (s *EnterpriseService) SetDefaultCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, newReposParam string) (*CodeSecurityConfigurationWithDefaultForNewRepos, *Response, error) { | |
| func (s *EnterpriseService) SetDefaultCodeSecurityConfiguration(ctx context.Context, enterprise string, id int64, defaultForNewRepos string) (*CodeSecurityConfigurationWithDefaultForNewRepos, *Response, error) { |
| // GitHub API docs: https://docs.github.com/rest/code-security/configurations#get-code-security-configurations-for-an-enterprise | ||
| // | ||
| //meta:operation GET /enterprises/{enterprise}/code-security/configurations | ||
| func (s *EnterpriseService) GetCodeSecurityConfigurations(ctx context.Context, enterprise string) ([]*CodeSecurityConfiguration, *Response, error) { |
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.
| // GitHub API docs: https://docs.github.com/rest/code-security/configurations#create-a-code-security-configuration-for-an-enterprise | ||
| // | ||
| //meta:operation POST /enterprises/{enterprise}/code-security/configurations | ||
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
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.
I think should be a separate struct (fields name and description are required):
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, c *CodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { | |
| func (s *EnterpriseService) CreateCodeSecurityConfiguration(ctx context.Context, enterprise string, config CreateCodeSecurityConfiguration) (*CodeSecurityConfiguration, *Response, error) { |
type CreateCodeSecurityConfiguration struct {
Name string `json:"name"`
Desription string `json:"description"`
// all other optional fields
}
BREAKING CHANGE:
AttachCodeSecurityConfigurationsToRepositoriesis nowAttachCodeSecurityConfigurationToRepositories.please let me know if I should drop the commit with breaking changes and what you think about the other commits