Skip to content
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

Add Webhook for Repository ruleset #3298

Conversation

unamdev0
Copy link

@unamdev0 unamdev0 commented Oct 2, 2024

Added Webhook for Repository ruleset

Copy link

google-cla bot commented Oct 2, 2024

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.

@gmlewis gmlewis changed the title Added Webhook for Repository ruleset Add Webhook for Repository ruleset Oct 2, 2024
@unamdev0 unamdev0 marked this pull request as ready for review October 4, 2024 11:53
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.05%. Comparing base (2b8c7fa) to head (a872d15).
Report is 132 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3298      +/-   ##
==========================================
- Coverage   97.72%   93.05%   -4.67%     
==========================================
  Files         153      172      +19     
  Lines       13390    11772    -1618     
==========================================
- Hits        13085    10955    -2130     
- Misses        215      723     +508     
- Partials       90       94       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unamdev0 unamdev0 force-pushed the feature/add-repository-ruleset-webhook-even branch from de359f4 to a872d15 Compare October 4, 2024 12:58
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

I notice that you are still pushing changes while I'm reviewing the code so I'm going to stop reviewing now since there are many changes that needs to be made.
Thanks.

@@ -1505,6 +1505,20 @@ type RepositoryImportEvent struct {
Sender *User `json:"sender,omitempty"`
}

// RepositoryRuleSetEvent event occurs when there is activity relating to repository rulesets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Godocs should always use complete sentences with punctuation so that the auto-generated package documentation formats and reads nicely.

Also, we should match the capitalization that GitHub uses for "ruleset" so everywhere that you wrote "RuleSet" it should instead be "Ruleset".

Suggested change
// RepositoryRuleSetEvent event occurs when there is activity relating to repository rulesets
// RepositoryRulesetEvent event occurs when there is activity relating to repository rulesets.

// RepositoryRuleSetEvent event occurs when there is activity relating to repository rulesets
//
// GitHub API docs: https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository_ruleset
type RepositoryRuleSetEvent struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type RepositoryRuleSetEvent struct {
type RepositoryRulesetEvent struct {

Installation *Installation `json:"installation,omitempty"`
Organization *Organization `json:"organization,omitempty"`
Repository *Repository `json:"repository,omitempty"`
RepositoryRuleSet *RepositoryRuleSet `json:"repository_ruleset"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
RepositoryRuleSet *RepositoryRuleSet `json:"repository_ruleset"`
RepositoryRuleset *RepositoryRuleset `json:"repository_ruleset"`

Organization *Organization `json:"organization,omitempty"`
Repository *Repository `json:"repository,omitempty"`
RepositoryRuleSet *RepositoryRuleSet `json:"repository_ruleset"`
Changes *RepositoryRuleSetEditedChanges `json:"changes,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Changes *RepositoryRuleSetEditedChanges `json:"changes,omitempty"`
Changes *RepositoryRulesetEditedChanges `json:"changes,omitempty"`

@@ -9448,6 +9448,1782 @@ func TestReleaseEvent_Marshal(t *testing.T) {
testJSONMarshal(t, u, want)
}

func TestRepositoryRuleSetEvent_Marshal(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func TestRepositoryRuleSetEvent_Marshal(t *testing.T) {
func TestRepositoryRulesetEvent_Marshal(t *testing.T) {

@@ -9448,6 +9448,1782 @@ func TestReleaseEvent_Marshal(t *testing.T) {
testJSONMarshal(t, u, want)
}

func TestRepositoryRuleSetEvent_Marshal(t *testing.T) {
testJSONMarshal(t, &RepositoryRuleSetEvent{}, "{}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
testJSONMarshal(t, &RepositoryRuleSetEvent{}, "{}")
testJSONMarshal(t, &RepositoryRulesetEvent{}, "{}")


jsonMsg, _ := json.Marshal(&l)

u := &RepositoryRuleSetEvent{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll stop making suggestions for these, but we should change them all to be consistent with GitHub's documentation.

Suggested change
u := &RepositoryRuleSetEvent{
u := &RepositoryRulesetEvent{

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 4, 2024

Please do NOT use force-push as described in CONTRIBUTING.md, especially on large PRs like this one, since that makes it extremely difficult for the reviewer to see what has changed since the last review. Thanks.

@unamdev0
Copy link
Author

unamdev0 commented Oct 4, 2024

review

Oh, I didn't I just rebased it, will make all the changes suggested and will let you know

@unamdev0 unamdev0 closed this Oct 4, 2024
@unamdev0 unamdev0 deleted the feature/add-repository-ruleset-webhook-even branch October 4, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants