-
Notifications
You must be signed in to change notification settings - Fork 176
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
Bugs in teams plugin #160
Comments
sorry that there have been so many discrepancies with these. i do really appreciate your help in investigating and resolving them. i do have an open issue specifically for an additional layer of testing that expands the scope to include the use of nock against actual api calls. the current tests stub out octokit/rest which brings the risk of not recognizing breaking changes in that library since they are stubbed in a way that no longer matches reality. as far as the actual plan goes, i need to find time to get at least one example test in place that follows this approach and get a chance to start to feel out if it should have any further impact on the overall testing strategy. its hard for me to make promises on timeline, though, since this is volunteer effort and i'm balancing against other obligations (including sickness going through our family currently). this is high on my list though, so i do hope to make progress soon. once i have an example test in place, would you be interested in contributing additional test coverage? |
@travi Yes I would like to help out |
great, thank you again. i'll be sure to keep you informed once i get a chance to make progress there, but also be sure to follow #145, if you aren't already |
Error output when changing/adding team permissions:
Empty value for parameter 'owner'
Empty value for parameter 'team_id'
GitHub request: PUT /teams/:team_id/repos/:owner/:repo - 400
I messed around with this setting when I created a dummy organization to test the settings. The field names here are incorrect.
id
should beteam_id
andorg
should beowner
. This is outlined in the documentation octokit/restI will open a PR to fix this. On a side note, is there a plan to write better coverage test cases with Nock to catch these issues?
The text was updated successfully, but these errors were encountered: