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

Fix `GitJSONDSL` and `diffForFile` for BitBucket Server #764

Merged
merged 1 commit into from Nov 15, 2018

Conversation

@langovoi
Copy link
Contributor

@langovoi langovoi commented Nov 14, 2018

Bitbucket Server API for diff sometimes return truncated: true in root of response.
This means that Bitbucket Server don't provide full diff of all files.

In my practical with Danger, this breaks modified_files, created_files and deleted_files.

Bitbucket Server has another API, which returns list of changes in PR, without diff.
I replaced diff API with changes API for generation GitJSONDSL.

Also I reworked diffForFile to fix potential bug with Bitbucket Server, when diff API may return truncated results.

@langovoi langovoi requested a review from orta Nov 14, 2018
@langovoi
Copy link
Contributor Author

@langovoi langovoi commented Nov 14, 2018

@orta could you review, please?


values = values.concat(data.values)
nextPageStart = data.nextPageStart
} while (nextPageStart !== null)

This comment has been minimized.

@langovoi

langovoi Nov 14, 2018
Author Contributor

Collect all pages with changes

@@ -3,7 +3,7 @@ import { dangerSignaturePostfix } from "../../../runner/templates/bitbucketServe

describe("API testing - BitBucket Server", () => {
let api: BitBucketServerAPI
let jsonResult: any
let jsonResult: () => any

This comment has been minimized.

@langovoi

langovoi Nov 14, 2018
Author Contributor

Allowing to set jsonResult as function. For example for use jest.fn

nextPageStart: 1,
values: ["1"],
})
.mockReturnValueOnce({

This comment has been minimized.

@langovoi

langovoi Nov 14, 2018
Author Contributor

Simulate iteration by pages

@orta
Copy link
Member

@orta orta commented Nov 15, 2018

OK, cool, yeah this looks good 👍

@orta orta merged commit 0afefa8 into danger:master Nov 15, 2018
3 checks passed
3 checks passed
Danger All good
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@langovoi langovoi deleted the langovoi:bitbucket-diff branch Nov 15, 2018
@langovoi
Copy link
Contributor Author

@langovoi langovoi commented Nov 15, 2018

Great! Could you make new release, please?

@orta
Copy link
Member

@orta orta commented Nov 15, 2018

Yep, this came out in 6.1.4

@sebinsua

This comment has been minimized.

Copy link
Contributor

@sebinsua sebinsua commented on source/danger.d.ts in f680b5f Dec 3, 2018

@langovoi Am I misunderstanding something or are you missing the "COPY" ChangeType?

At the firm I work at we are getting "Unhandled change type" errors and this is my first suspicion.

This comment has been minimized.

Copy link
Contributor Author

@langovoi langovoi replied Dec 3, 2018

Oh..sure, "COPY" must be like "ADD" in bitBucketServerChangesToGitJSONDSL

This comment has been minimized.

Copy link
Contributor Author

@langovoi langovoi replied Dec 3, 2018

Could you make PR?

This comment has been minimized.

Copy link
Contributor

@sebinsua sebinsua replied Dec 3, 2018

Yeah, sure. I will have a try when I get home! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants