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 core functionality for Project API's #160

Merged
merged 8 commits into from
May 11, 2020

Conversation

jpomfret
Copy link
Contributor

@jpomfret jpomfret commented Apr 26, 2020

Adding support for projects core functionality

  • Get
    • Repository projects
    • Organization projects
    • User Projects
    • Any of the above by Project Id
  • New
    • Repository projects
    • Organization projects
    • User Projects
  • Set
    • By Id - but can pass in information for repo, organisation or user
  • Delete
    • By Id - but can pass in information for repo, organisation or user

Also added test coverage - all tests passed on my machine, I did have issues with the last call to remove the repository.

Resolves #159

@jpomfret
Copy link
Contributor Author

Issue was because I didn't give my token the right access :)

@HowardWolosky HowardWolosky changed the title Add core project functionality for issue #159 Add core project functionality Apr 27, 2020
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.

Wow....this is GREAT! Thanks so much for putting in all this great work. Very much appreciated! I'm sure that this will help out a bunch of users.

I think that you did a great job adhering to the overall project style. I've left a number of comments for you to review and address.

Again...thanks so much for all of this!

GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Show resolved Hide resolved
GitHubProjects.ps1 Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
Tests/GitHubProjects.tests.ps1 Show resolved Hide resolved
$results = Get-GitHubProject -Project $repoProject.id
}
catch {
$null
Copy link
Member

Choose a reason for hiding this comment

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

What's the scenario you're working around here? At the very least, this deserves a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Get was failing to find the project and when using the -Project parameter it threw an error which messed up the tests.

Changed these tests around to test for the thrown error instead... new version incoming :)

@jpomfret
Copy link
Contributor Author

Thank you so much for the code review! Working through the changes.

@jpomfret
Copy link
Contributor Author

force pushed because I made the same changes in my local repo as I committed from the review (didn't realise what I was doing was actually making the changes, sorry if that broke anything!)

@jpomfret
Copy link
Contributor Author

jpomfret commented May 4, 2020

Hey @HowardWolosky - I have another piece of work ready to PR for Project Columns, wondering if I should lump it into this PR or if you'd rather keep it seperate?

Also - let me know if I'm still outstanding on any review items for this core project piece - I believe everything has been addressed.

Thanks!

@HowardWolosky
Copy link
Member

@jpomfret, please keep them separate. I'd also recommend, if possible, to do it in a new branch off of master as opposed to a new branch off of jpomfret:projects. Things will get a bit trickier if you continue to branch off new work from a branch that's in review.

I'll be continuing my review of this PR tomorrow.

Thanks again for the contributions....Looking forward to seeing the additional work!

@jpomfret
Copy link
Contributor Author

jpomfret commented May 4, 2020

Thanks @HowardWolosky - I submitted two new PR's for the other pieces. I'll be around tomorrow for any fixes you need. (I tried really hard on spacing and brackets.. but I'm sure you'll find some 😄)

@jpomfret jpomfret mentioned this pull request May 7, 2020
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.

Thanks for the update.
Just a few minor additional corrections to make, then we can merge this in.

GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Outdated Show resolved Hide resolved
GitHubProjects.ps1 Show resolved Hide resolved
Tests/GitHubProjects.tests.ps1 Outdated Show resolved Hide resolved
@HowardWolosky HowardWolosky self-assigned this May 11, 2020
@HowardWolosky HowardWolosky added api completeness This is basic API functionality that hasn't been implemented yet. api-project Work to complete the API's defined here: https://developer.github.com/v3/projects labels May 11, 2020
@HowardWolosky HowardWolosky changed the title Add core project functionality Add core functionality for Project API's May 11, 2020
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.

Final updates look good.
Locally ran all tests and they passed.
This looks good to merge.

🎆 🚢 🎆

@HowardWolosky HowardWolosky merged commit 1cdaac1 into microsoft:master May 11, 2020
@HowardWolosky
Copy link
Member

This is now available publicly in 0.12.0. Thanks so much for your contribution. 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api completeness This is basic API functionality that hasn't been implemented yet. api-project Work to complete the API's defined here: https://developer.github.com/v3/projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Completeness - Projects
2 participants