Skip to content

Added Discussions support #382

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

Merged
merged 5 commits into from
Feb 26, 2023
Merged

Added Discussions support #382

merged 5 commits into from
Feb 26, 2023

Conversation

variableresistor
Copy link
Contributor

@variableresistor variableresistor commented Dec 16, 2022

Description

Allow the user to enable Discussions in Github repositories

Issues Fixed

Fixes #378

References

https://docs.github.com./en/rest/repos/repos?apiVersion=2022-11-28#update-a-repository

Checklist

  • You actually ran the code that you just wrote, especially if you did just "one last quick change".
  • [ x] Comment-based help added/updated, including examples.
  • Static analysis is reporting back clean.
  • New/changed code adheres to our coding guidelines.
    - [ ] Formatters were created for any new types being added.
  • New/changed code continues to support the pipeline.
    - [ ] Changes to the manifest file follow the manifest guidance.
  • [] Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
    - [ ] Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

@variableresistor
Copy link
Contributor Author

variableresistor commented Dec 16, 2022

Turns out unit tests aren't working. My mistake. Getting:
[-] Context GitHubRepositories\New-GitHubRepository.When creating a repository for the authenticated user.When creating a private repository with default settings failed
[0] HttpResponseException: Response status code does not indicate success: 404 (Not Found).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at Remove-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:525
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:103
[1] HttpResponseException: Response status code does not indicate success: 500 (Internal Server Error).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at New-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:233
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:77
[-] Context GitHubRepositories\New-GitHubRepository.When creating an organization repository.When creating a public repository with default settings failed
[0] HttpResponseException: Response status code does not indicate success: 404 (Not Found).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at Remove-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:525
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:253
[1] HttpResponseException: Response status code does not indicate success: 404 (Not Found).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at New-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:233
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:226
[-] Context GitHubRepositories\New-GitHubRepository.When creating an organization repository.When creating a private repository with default settings failed
[0] HttpResponseException: Response status code does not indicate success: 404 (Not Found).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at Remove-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:525
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:292
[1] HttpResponseException: Response status code does not indicate success: 404 (Not Found).
at Invoke-GHRestMethod, C:\Repos\PowerShellForGitHub\GitHubCore.ps1:320
at New-GitHubRepository, C:\Repos\PowerShellForGitHub\GitHubRepositories.ps1:233
at , C:\Repos\PowerShellForGitHub\Tests\GitHubRepositories.tests.ps1:266

Strangely, some tests are still passing:
Tests completed in 33.74s
Tests Passed: 9, Failed: 6, Skipped: 0 NotRun: 133
BeforeAll \ AfterAll failed: 3

  • GitHubRepositories\New-GitHubRepository.When creating a repository for the authenticated user.When creating a private repository with default settings
  • GitHubRepositories\New-GitHubRepository.When creating an organization repository.When creating a public repository with default settings
  • GitHubRepositories\New-GitHubRepository.When creating an organization repository.When creating a private repository with default settings

@variableresistor
Copy link
Contributor Author

Works okay when I run this:

$Repo = New-GitHubRepository -Name (New-Guid) -Discussions
$Repo.has_discussions
True

Been going in circles on this for an hour. Read all the contributing docs in the repo. Doesn't even work when I revert my change.

@HowardWolosky
Copy link
Member

Been going in circles on this for an hour. Read all the contributing docs in the repo. Doesn't even work when I revert my change.

It looks like you're not sync'd up with what's currently in main.

@variableresistor
Copy link
Contributor Author

Been going in circles on this for an hour. Read all the contributing docs in the repo. Doesn't even work when I revert my change.

It looks like you're not sync'd up with what's currently in main.

Yeah, that makes sense; I'm probably not getting all the Pester 5+ changes. My repo says it's up to date with the main branch, but it's clearly not. So can I just close out this PR, re-create my fork, then open a new one?

@HowardWolosky
Copy link
Member

Yeah, that makes sense; I'm probably not getting all the Pester 5+ changes. My repo says it's up to date with the main branch, but it's clearly not. So can I just close out this PR, re-create my fork, then open a new one?

That's overkill. No need to do that. You just need to merge the current changes in master .

First, add a named branch for upstream:

git remote add upstream https://github.com./microsoft/PowerShellForGitHub.git

After that, it's easy to do operations in your fork that interact with this project. This will get your fork's master fully in-sync with this one.

git checkout master
git pull upstream master
git push

Finally, just rebase your branch on top of your local master

git checkout <your local branch name>
git rebase master
git push -f

After that, your fork will be modifying on top of the current change in this project's master.

@variableresistor
Copy link
Contributor Author

@HowardWolosky okay, my change is actually working here. It's in the context "When creating a repository with all possible settings". It's the context "When creating a private repository with default settings" that's not working. It uses the same function to run both, but my new -Discussions parameter isn't even being added to the header. So I'm starting to believe it's a different issue. Here's some relevent variables I inspected during the test run:

$params
Name                           Value
----                           -----
TelemetryEventName             New-GitHubRepository
Method                         Post
UriFragment                    user/repos
AcceptHeader                   application/vnd.github.baptiste-preview+json
TelemetryProperties            {RepositoryName}
Body                           {…
Description                    Creating 359474aa-4bfa-4fc2-8718-d60b1bea7626
AccessToken

PS C:\Repos\PowerShellForGitHub> $params.Body
{
  "name": "359474aa-4bfa-4fc2-8718-d60b1bea7626",
  "private": true
}

Line 222 in GitHubRepositories.ps1 while running Context -Name 'When creating a private repository with default settings'

Error:
HttpResponseException: Response status code does not indicate success: 500 (Internal Server Error)

@HowardWolosky
Copy link
Member

I'm not seeing the test failures that you're describing when running your changes locally.

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Pretty straightforward change. Just looking for a change to the parameter name.

Also, as a tip for the future -- you may find it easier to work on your changes in branches that you then submit pull requests, as opposed to working directly in your fork's master. That would allow you to work on multiple unrelated changes at once (which is the problem you were previously running into).

@ghost ghost added the needs-author-feedback label Jan 5, 2023
@X-Guardian
Copy link
Contributor

This parameter should also be added to the Set-GitHubRepository function.

@ghost
Copy link

ghost commented Jan 18, 2023

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 14 days of this comment.

@variableresistor
Copy link
Contributor Author

This parameter should also be added to the Set-GitHubRepository function.

This would make sense, but the GitHub docs don't show has_discussions as a parameter to update a repository.

@X-Guardian
Copy link
Contributor

No, but it does work (I tried it). I think the documentation is incomplete.

@variableresistor
Copy link
Contributor Author

No, but it does work (I tried it). I think the documentation is incomplete.

I pushed the change right before I saw your comment. Changing the other as well.

@HowardWolosky
Copy link
Member

/azp run PowerShellForGitHub-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Update looks good. Thanks for the contribution!

@HowardWolosky HowardWolosky merged commit 3ccee43 into microsoft:master Feb 26, 2023
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.

Add toggle for Discussions in Repositories
3 participants