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 support for milestones #62

Merged
merged 9 commits into from
Dec 13, 2018
Merged

Add support for milestones #62

merged 9 commits into from
Dec 13, 2018

Conversation

etgottli
Copy link
Contributor

@etgottli etgottli commented Dec 5, 2018

Added functionality for the milestones api

GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
Tests/GitHubMilestones.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubMilestones.tests.ps1 Show resolved Hide resolved
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 contribution! I've noted some requested changes for you to make.

GitHubMilestones.ps1 Outdated Show resolved Hide resolved
USAGE.md Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
Tests/GitHubMilestones.tests.ps1 Outdated Show resolved Hide resolved
danbelcher-MSFT
danbelcher-MSFT previously approved these changes Dec 6, 2018
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
Tests/GitHubMilestones.tests.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
GitHubMilestones.ps1 Outdated Show resolved Hide resolved
USAGE.md Show resolved Hide resolved
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.

Good update. Minor issues remaining.

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 updates. Minor changes requested. Will be interested in hearing from @danbelcher-MSFT as well.

GitHubMilestones.ps1 Show resolved Hide resolved
GitHubMilestones.ps1 Show resolved Hide resolved
$existingMilestone.title | Should be $defaultMilestoneTitle1
}

It "Should have the expected state" {

Choose a reason for hiding this comment

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

My nitpick here is that these test names are generic and so don't tell you anything about what is actually being tested. When you run the tests, you should see:

For creating a new milestone:
    ...
    'Should be in the closed state'

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 contribution!

@HowardWolosky
Copy link
Member

Can you please get this branch up-to-date with what's in master? There are currently conflicts preventing the merge.

@etgottli
Copy link
Contributor Author

@HowardWolosky I had merged master into this branch before you made that conflicts comment, and I'm not seeing any conflicts anymore. The only thing blocking the merge now is that "The base branch restricts merging to authorized users" and I'm not authorized. I assume you are able to merge the PR?

@HowardWolosky HowardWolosky merged commit 2bd2447 into master Dec 13, 2018
@HowardWolosky
Copy link
Member

Thanks -- issue was on my end. GitHub was saying that I couldn't rebase your changes on top of master due to merge conflicts, but it had no problem just squashing your changes and then merging on top of master.

@HowardWolosky HowardWolosky deleted the etgottli/milestones branch April 9, 2019 15:24
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.

4 participants