Bitbucket platform updates / bug fixes #632
Conversation
markkemper1
commented
Jan 2, 2019
markkemper1
commented
- Support seperate username and repository owner, needed if repository is owned by a team
- Directly queries a repository rather then fetch all and filter on the client, filtering on the client doesn't work as the get all implementation only returns the first page of results. (its also inefficient approach)
… https://markkemper@bitbucket.org/my-team/fancy-repo.git , also strips .git extension when parsing the repository name from the url
…an query for 'all' and then filter client side. Currenty 'all' implementation only fetches first page of results
| @@ -57,7 +63,7 @@ public async Task<IEnumerable<Ref>> GetRepositoryRefs(string account, string rep | |||
| public async Task<PullRequest> CreatePullRequest(PullRequest request, string account, string reponame) | |||
| { | |||
| var response = await _client.PostAsync(($"repositories/{account}/{reponame}/pullrequests"), | |||
| new StringContent(JsonConvert.SerializeObject(request, Formatting.None, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore}), Encoding.UTF8, "application/json")); | |||
| new StringContent(JsonConvert.SerializeObject(request, Formatting.None, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore }), Encoding.UTF8, "application/json")); | |||
There was a problem hiding this comment.
I would keep the JsonSerializerSettings as a private static readonly value. it's not going to change between calls.
There was a problem hiding this comment.
There isn't a benefit in moving this into a variable as this is the only place its used in the file. So it will just be a level of indirection (and more code) that we don't need. If it was used in multiple places then I think thats the time to introduce a variable.
On a side node, I didn't actually change this code, my editor just inserted a space and it wasn't easy to remove it (without turning off the auto formatting on my editor ;) ...
Happy to leave it better than I found it, just not seeing much value in this change.
There was a problem hiding this comment.
I've moved the settings into a field.
| { | ||
| return new Repository(repo.name, false, | ||
| new UserPermissions(true, true, true), | ||
| new Uri(repo.links.clone.First().href), |
There was a problem hiding this comment.
I would extract the repo.links.clone.First().href as a local var, that expression only needs to be evaluated once.
There was a problem hiding this comment.
I just moved this code into a method to avoid repeating it.
The two links are actually different things so I feel a bit icky moving that into a variable, if you could let me know how the two urls are used, maybe I could actually make them different.
a) HtmlUrl - is this a browser url that links you to the repo homepage?
b) CloneUrl - assuming this is the https (or ssh) clone url
Bitbucket actually sends back the following urls:
"links": {
"watchers": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/watchers"
},
"branches": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/refs/branches"
},
"tags": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/refs/tags"
},
"commits": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/commits"
},
"clone": [
{
"href": "https://username@bitbucket.org/owner-slug/repostiory-slug.git",
"name": "https"
},
{
"href": "git@bitbucket.org:owner-slug/repostiory-slug.git",
"name": "ssh"
}
],
"self": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug"
},
"source": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/src"
},
"html": {
"href": "https://bitbucket.org/owner-slug/repostiory-slug"
},
"avatar": {
"href": "https://bytebucket.org/ravatar/%7B315caa4f-5259-4ec8-b789-446ae523f770%7D?ts=2081905"
},
"hooks": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/hooks"
},
"forks": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/forks"
},
"downloads": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/downloads"
},
"pullrequests": {
"href": "https://api.bitbucket.org/2.0/repositories/owner-slug/repostiory-slug/pullrequests"
}So maybe we use the "html" link for HtmlUrl and the "clone[name=https]" link for CloneUrl ?
There was a problem hiding this comment.
Yes, CloneUrl must be the http/s (not ssh) url that we use to pull from git.
HtmlUrl ... might be some cruft here, it might not be needed. it's used similarly.
There was a problem hiding this comment.
Updated the links to use "html" link for HtmlUrl and the "clone[name=https]" link for CloneUrl
|
Looks generally ok to me, but I'm not a bitbucket expert. |
|
Well i wrote this code a few months ago (which was heavily based on the vsts intergration) but i'm by no means a Bitbucket expert. I've tested it on my bitbucket repo's and it worked fine then. Would be great if we could get somebody experienced with Bitbucket help out with the NuKeeper intergration every once in a while.. |
