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.

Sign up for GitHub

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

Adds MaintainerCanModify to PullRequest #1771

Merged
merged 12 commits into from
Apr 23, 2018
Merged

Adds MaintainerCanModify to PullRequest #1771

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
67bb3f8
Adds MaintainerCanModify to PullRequest
Cyberboss Feb 25, 2018
ea59264
Make MaintainerCanModify nullable
Cyberboss Feb 26, 2018
901859e
Tweak MaintainerCanModify doc comment
Cyberboss Feb 26, 2018
2cb4e5f
Add MaintainerCanModify to NewPullRequest
Cyberboss Feb 26, 2018
8276b1e
Add MaintainerCanModify to PullRequestUpdate
Cyberboss Feb 27, 2018
f20dfa7
Update NewPulRequest tests
Cyberboss Feb 27, 2018
6d72c42
Convert tabs to spaces
Cyberboss Feb 27, 2018
04c829e
Fix PullRequestUpdate and NewPullRequest doc comments
Cyberboss Feb 27, 2018
6c2c31e
Revert "Update NewPulRequest tests"
Cyberboss Mar 5, 2018
56819df
Make MaintainerCanModify fields nullable
Cyberboss Mar 5, 2018
f72b218
tweak doco comment and make setter public so it can be used
ryangribble Apr 16, 2018
3328977
Merge remote-tracking branch 'upstream/master' into patch-1
ryangribble Apr 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Octokit.Tests.Integration/Clients/PullRequestReviewCommentReactionsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ async Task<PullRequestData> CreatePullRequest(RepositoryContext context)

// Creating a pull request

var pullRequest = new NewPullRequest("Nice title for the pull request", branchName, "master");
var pullRequest = new NewPullRequest("Nice title for the pull request", branchName, "master", false);
var createdPullRequest = await _github.PullRequest.Create(Helper.UserName, repoName, pullRequest);

var data = new PullRequestData
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests.Integration/Clients/PullRequestReviewCommentsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ async Task<PullRequestData> CreatePullRequest(RepositoryContext context, string

// Creating a pull request

var pullRequest = new NewPullRequest("Nice title for the pull request", branch, "master");
var pullRequest = new NewPullRequest("Nice title for the pull request", branch, "master", false);
var createdPullRequest = await _github.PullRequest.Create(Helper.UserName, repoName, pullRequest);

var data = new PullRequestData
Expand Down
2 changes: 1 addition & 1 deletion Octokit.Tests.Integration/Clients/PullRequestReviewRequestsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ static async Task<int> CreateTheWorld(IGitHubClient github, RepositoryContext co
await github.Git.Reference.Create(context.RepositoryOwner, context.RepositoryName, new NewReference("refs/heads/my-branch", featureBranchCommit2.Sha));

// create a pull request
var pullRequest = new NewPullRequest("Nice title for the pull request", "my-branch", "master");
var pullRequest = new NewPullRequest("Nice title for the pull request", "my-branch", "master", false);
var createdPullRequest = await github.PullRequest.Create(context.RepositoryOwner, context.RepositoryName, pullRequest);

// Create review requests (optional)
Expand Down
110 changes: 56 additions & 54 deletions Octokit.Tests.Integration/Clients/PullRequestsClientTests.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Octokit.Tests.Integration/Helpers/RepositorySetupHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static async Task<Reference> CreateTheWorld(this IGitHubClient client, Re

public static async Task<PullRequest> CreatePullRequest(this IGitHubClient client, Repository repository, string branch = "my-branch")
{
var pullRequest = new NewPullRequest("Nice title for the pull request", branch, "master");
var pullRequest = new NewPullRequest("Nice title for the pull request", branch, "master", false);
var createdPullRequest = await client.PullRequest.Create(repository.Owner.Login, repository.Name, pullRequest);

return createdPullRequest;
Expand Down
20 changes: 10 additions & 10 deletions Octokit.Tests/Clients/PullRequestsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public class TheCreateMethod
[Fact]
public async Task PostsToCorrectUrl()
{
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name");
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name", false);
var connection = Substitute.For<IApiConnection>();
var client = new PullRequestsClient(connection);

Expand All @@ -249,7 +249,7 @@ public async Task PostsToCorrectUrl()
[Fact]
public async Task PostsToCorrectUrlWithRepositoryId()
{
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name");
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name", false);
var connection = Substitute.For<IApiConnection>();
var client = new PullRequestsClient(connection);

Expand All @@ -265,14 +265,14 @@ public async Task EnsuresNonNullArguments()
var connection = Substitute.For<IApiConnection>();
var client = new PullRequestsClient(connection);

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", "name", null));

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(1, null));

await Assert.ThrowsAsync<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2", false)));
}
}

Expand Down Expand Up @@ -310,14 +310,14 @@ public async Task EnsuresNonNullArguments()
var connection = Substitute.For<IApiConnection>();
var client = new PullRequestsClient(connection);

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create("owner", "name", null));

await Assert.ThrowsAsync<ArgumentNullException>(() => client.Create(1, null));

await Assert.ThrowsAsync<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2")));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2", false)));
await Assert.ThrowsAsync<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2", false)));
}
}

Expand Down
12 changes: 6 additions & 6 deletions Octokit.Tests/Reactive/ObservablePullRequestsClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public class TheCreateMethod
[Fact]
public void CreatesFromClientRepositoryPullRequest()
{
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name");
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name", false);
var gitHubClient = Substitute.For<IGitHubClient>();
var client = new ObservablePullRequestsClient(gitHubClient);

Expand All @@ -474,7 +474,7 @@ public void CreatesFromClientRepositoryPullRequest()
[Fact]
public void CreatesFromClientRepositoryPullRequestWithRepositoryId()
{
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name");
var newPullRequest = new NewPullRequest("some title", "branch:name", "branch:name", false);
var gitHubClient = Substitute.For<IGitHubClient>();
var client = new ObservablePullRequestsClient(gitHubClient);

Expand All @@ -489,14 +489,14 @@ public void EnsuresNonNullArguments()
var gitHubClient = Substitute.For<IGitHubClient>();
var client = new ObservablePullRequestsClient(gitHubClient);

Assert.Throws<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2")));
Assert.Throws<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2")));
Assert.Throws<ArgumentNullException>(() => client.Create(null, "name", new NewPullRequest("title", "ref", "ref2", false)));
Assert.Throws<ArgumentNullException>(() => client.Create("owner", null, new NewPullRequest("title", "ref", "ref2", false)));
Assert.Throws<ArgumentNullException>(() => client.Create("owner", "name", null));

Assert.Throws<ArgumentNullException>(() => client.Create(1, null));

Assert.Throws<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2")));
Assert.Throws<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2")));
Assert.Throws<ArgumentException>(() => client.Create("", "name", new NewPullRequest("title", "ref", "ref2", false)));
Assert.Throws<ArgumentException>(() => client.Create("owner", "", new NewPullRequest("title", "ref", "ref2", false)));
}
}

Expand Down
9 changes: 8 additions & 1 deletion Octokit/Models/Request/NewPullRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ public class NewPullRequest
/// <param name="title">The title of the pull request.</param>
/// <param name="head">The branch (or git ref where your changes are implemented. In other words, the source branch/ref</param>
/// <param name="baseRef">The base (or git ref) reference you want your changes pulled into. In other words, the target branch/ref</param>
public NewPullRequest(string title, string head, string baseRef)
/// <param name="maintainerCanModify">Whether maintainers of the base repository can push to <paramref name="head"/> if it is a branch</param>
public NewPullRequest(string title, string head, string baseRef, bool maintainerCanModify)
Copy link
Contributor

@ryangribble ryangribble Feb 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

we only want mandatory parameters to be in the constructor, so this can be removed sorry! Good news is, all those tests don't need to be modified (and the bad news is you have to undo all the test modifications LOL)

{
Ensure.ArgumentNotNullOrEmptyString(title, "title");
Ensure.ArgumentNotNullOrEmptyString(head, "head");
Expand All @@ -24,6 +25,7 @@ public NewPullRequest(string title, string head, string baseRef)
Title = title;
Head = head;
Base = baseRef;
MaintainerCanModify = maintainerCanModify;
}

/// <summary>
Expand All @@ -41,6 +43,11 @@ public NewPullRequest(string title, string head, string baseRef)
/// </summary>
public string Head { get; private set; }

/// <summary>
/// Whether maintainers of the base repository can push to <see cref="Head"/> if it is a branch
/// </summary>
public bool MaintainerCanModify { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be nullable bool? since it's an optional parameter. That way when the user doesnt set it, we dont send anything in the request, and thus we default to whatever the API default is (which is important in this case since the API default is true, and if we leave this non nullable we would default to sending false)


/// <summary>
/// Body of the pull request (optional)
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions Octokit/Models/Request/PullRequestUpdate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public class PullRequestUpdate
/// </summary>
public string Base { get; set; }

/// <summary>
/// Whether maintainers of the base repository can push to the pull request's HEAD if it is a branch
/// </summary>
public bool MaintainerCanModify { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be nullable bool? so that the user is able to not specify a value and have it not change this property on the PR they are updating


internal string DebuggerDisplay
{
get
Expand Down
8 changes: 7 additions & 1 deletion Octokit/Models/Response/PullRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public PullRequest(int number)
Number = number;
}

public PullRequest(long id, string url, string htmlUrl, string diffUrl, string patchUrl, string issueUrl, string statusesUrl, int number, ItemState state, string title, string body, DateTimeOffset createdAt, DateTimeOffset updatedAt, DateTimeOffset? closedAt, DateTimeOffset? mergedAt, GitReference head, GitReference @base, User user, User assignee, IReadOnlyList<User> assignees, bool? mergeable, MergeableState? mergeableState, User mergedBy, string mergeCommitSha, int comments, int commits, int additions, int deletions, int changedFiles, Milestone milestone, bool locked, IReadOnlyList<User> requestedReviewers)
public PullRequest(long id, string url, string htmlUrl, string diffUrl, string patchUrl, string issueUrl, string statusesUrl, int number, ItemState state, string title, string body, DateTimeOffset createdAt, DateTimeOffset updatedAt, DateTimeOffset? closedAt, DateTimeOffset? mergedAt, GitReference head, GitReference @base, User user, User assignee, IReadOnlyList<User> assignees, bool? mergeable, MergeableState? mergeableState, User mergedBy, string mergeCommitSha, int comments, int commits, int additions, int deletions, int changedFiles, Milestone milestone, bool locked, bool? maintainerCanModify, IReadOnlyList<User> requestedReviewers)
{
Id = id;
Url = url;
Expand Down Expand Up @@ -49,6 +49,7 @@ public PullRequest(long id, string url, string htmlUrl, string diffUrl, string p
ChangedFiles = changedFiles;
Milestone = milestone;
Locked = locked;
MaintainerCanModify = maintainerCanModify;
RequestedReviewers = requestedReviewers;
}

Expand Down Expand Up @@ -219,6 +220,11 @@ public bool Merged
/// </summary>
public bool Locked { get; protected set; }

/// <summary>
/// Whether maintainers of the base repository can push to the HEAD branch
/// </summary>
public bool? MaintainerCanModify { get; protected set; }

/// <summary>
/// Users requested for review
/// </summary>
Expand Down