Skip to content

- added 'RolePermission' to allow specifying custom roles#1403

Merged
bitwiseman merged 3 commits intohub4j:mainfrom
jgangemi:jae/role-perm
Apr 8, 2022
Merged

- added 'RolePermission' to allow specifying custom roles#1403
bitwiseman merged 3 commits intohub4j:mainfrom
jgangemi:jae/role-perm

Conversation

@jgangemi
Copy link
Contributor

better fix for #1377, tests after this is ok-ed.

@bitwiseman didn't there used to be some way to format the code through maven?

@bitwiseman
Copy link
Member

Build output says it:

Error: 

Error: @@ -1162,7 +1163,7 @@ 

Error: ... (8 more lines that didn't fit) 

Error: Violations also present in: 

Error: src/main/java/org/kohsuke/github/GHTeam.java 

Error: Run 'mvn spotless:apply' to fix these violations.

@bitwiseman
Copy link
Member

Could send a link to the documentation for the endpoint and maybe any blog post announcing the change? It would help me understand the new behavior vs the old.

@jgangemi
Copy link
Contributor Author

basically instead of just having the pre-defined roles like before, you can create your own custom role that has additional permissions. for example, i created a developer role at the day job that grants ppl access to dependabot alerts.

i'm trying to find the documentation for the endpoint, but i know it's listed there.

@jgangemi
Copy link
Contributor Author

https://docs.github.com/en/rest/reference/teams#add-or-update-team-repository-permissions--parameters

this allows for the "custom" role to be defined and also provides the "pre-canned".

@jgangemi
Copy link
Contributor Author

@bitwiseman ping :) - i'd like to get this nailed down as it's something i need.

Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Basically yes, just some feedback.

@bitwiseman bitwiseman self-requested a review March 17, 2022 05:00
@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #1403 (2b6f549) into main (06f2f8b) will increase coverage by 0.18%.
The diff coverage is 85.00%.

@@             Coverage Diff              @@
##               main    #1403      +/-   ##
============================================
+ Coverage     78.22%   78.40%   +0.18%     
- Complexity     2073     2079       +6     
============================================
  Files           201      201              
  Lines          6341     6354      +13     
  Branches        357      357              
============================================
+ Hits           4960     4982      +22     
+ Misses         1172     1164       -8     
+ Partials        209      208       -1     
Impacted Files Coverage Δ
src/main/java/org/kohsuke/github/GHRepository.java 68.24% <57.14%> (+0.20%) ⬆️
...c/main/java/org/kohsuke/github/GHOrganization.java 66.20% <100.00%> (+1.45%) ⬆️
src/main/java/org/kohsuke/github/GHTeam.java 73.80% <100.00%> (+10.84%) ⬆️
src/main/java/org/kohsuke/github/GHPersonSet.java 42.85% <0.00%> (+7.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06f2f8b...2b6f549. Read the comment docs.

@jgangemi
Copy link
Contributor Author

@bitwiseman do you want me to work those coverage numbers up or is this good to go?

@jgangemi
Copy link
Contributor Author

@bitwiseman ping :)

Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

@jgangemi
I can see that the previous functionality wasn't covered well, but yes, we need to bring the coverage numbers up before merging. Add some more tests covering the deprecated methods as well. Thanks!

@jgangemi
Copy link
Contributor Author

jgangemi commented Apr 6, 2022

@bitwiseman good news/bad news! i got the coverage numbers up but now the build is failing and complaining about some missing constructor.

i didn't make any changes here, so i'm confused as to what is going on.

@bitwiseman
Copy link
Member

bitwiseman commented Apr 7, 2022

@jgangemi
Yeah, that is strange.

@jgangemi
Copy link
Contributor Author

jgangemi commented Apr 7, 2022

welp, looks like it's green now. can i get a merge and release? :)

@bitwiseman bitwiseman merged commit 68aeab2 into hub4j:main Apr 8, 2022
@jgangemi jgangemi deleted the jae/role-perm branch April 8, 2022 10:12
@jgangemi
Copy link
Contributor Author

@bitwiseman what happened to the release? can i get another cut?

https://github.com/hub4j/github-api/releases/tag/github-api-1.304

@bitwiseman
Copy link
Member

  • Nothing happened to the release...

Oh, right, sorry. I clicked the wrong button and GitHub marked a release as published. I wasn't ready to do one though so I rolled it back.

I'll release 1.304 when I'm back from vacation this Friday.

@jgangemi
Copy link
Contributor Author

thanks! enjoy!!!

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