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

Make PullRequestFile#patch an Option[String] #181

Conversation

lloydmeta
Copy link
Contributor

@lloydmeta lloydmeta commented Dec 5, 2017

Closes #180

That field is not populated on binary files.

Example: $ curl https://api.github.com/repos/scala/scala/pulls/4877/files | jq '. | map(select(.patch == null))'

[
  {
    "sha": "7229603ae5b30ce0e0bd09863543b260085c8f2d",
    "filename": "src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "status": "removed",
    "additions": 0,
    "deletions": 0,
    "changes": 0,
    "blob_url": "https://github.com/scala/scala/blob/da3720e55b3d69cc31ab0f26e6cffafb18da360f/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "raw_url": "https://github.com/scala/scala/raw/da3720e55b3d69cc31ab0f26e6cffafb18da360f/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "contents_url": "https://api.github.com/repos/scala/scala/contents/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png?ref=da3720e55b3d69cc31ab0f26e6cffafb18da360f"
  },
  ...
]

That field is not populated on binary files.

Example: curl https://api.github.com/repos/scala/scala/pulls/4877/files | jq '. | map(select(.patch == null))'

```json
[
  {
    "sha": "7229603ae5b30ce0e0bd09863543b260085c8f2d",
    "filename": "src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "status": "removed",
    "additions": 0,
    "deletions": 0,
    "changes": 0,
    "blob_url": "https://github.com/scala/scala/blob/da3720e55b3d69cc31ab0f26e6cffafb18da360f/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "raw_url": "https://github.com/scala/scala/raw/da3720e55b3d69cc31ab0f26e6cffafb18da360f/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png",
    "contents_url": "https://api.github.com/repos/scala/scala/contents/src/scaladoc/scala/tools/nsc/doc/html/resource/lib/arrow-down.png?ref=da3720e55b3d69cc31ab0f26e6cffafb18da360f"
  },
  ...
]
```
Copy link
Contributor

@BenFradet BenFradet left a comment

Choose a reason for hiding this comment

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

Looks good 👍 , I'll try fixing the build later today or tomorrow.

Copy link
Member

@juanpedromoreno juanpedromoreno left a comment

Choose a reason for hiding this comment

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

Thanks @lloydmeta ! LGTM

@lloydmeta
Copy link
Contributor Author

Thanks guys :)

I know this is a strange request, but is it at all possible to back-port this ti 0.15.x? The reason I ask is because it would be great to use this plugin from Ammonite but currently that isn't possible due to com-lihaoyi/Ammonite#718. Or if there is some workaround that you know of for using this lib w/ Ammonite, please let me know :)

@juanpedromoreno
Copy link
Member

Yeah, we are dealing with the same issue here 47degrees/sbt-org-policies#655

No workarounds yet :(

@juanpedromoreno
Copy link
Member

Regarding the build issue, this might fix the issue 47degrees/sbt-microsites#246

@lloydmeta
Copy link
Contributor Author

@juanpedromoreno good to know it's a known issue. My current thought is that maybe ideally sbt should be shading its depenencies on Jawn...

@guersam
Copy link
Contributor

guersam commented Dec 7, 2017

It seems the same as I experienced in doobie recently. #182 fixes the build error.

@juanpedromoreno
Copy link
Member

@lloydmeta agreed. It seems you found our attempt to do these changes on the sbt side:

sbt/serialization#21
eed3si9n/sjson-new#85

@juanpedromoreno juanpedromoreno merged commit 9452826 into 47degrees:master Dec 7, 2017
@lloydmeta lloydmeta deleted the fix/make-PullRequestFile-patch-optional branch December 14, 2017 05:41
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.

None yet

4 participants