search

timwis / jkan

A lightweight, backend-free open data portal, powered by Jekyll
https://jkan.io
MIT License
219 stars 309 forks source link

github-api version problems #124

Closed wilsaj closed 8 years ago

wilsaj commented 8 years ago

Hey there! We're working on a project to set up a slightly-customized version of JKAN. It looks like building the JS bundle from scratch doesn't quite work at the moment, because of a failure at this line: https://github.com/timwis/jkan/blob/gh-pages/scripts/src/models/user.js#L91

The reason is that the version 0.11.2 of the github-api library that JKAN wants to use doesn't contain Repository.isCollaborators(). That was added after 0.11.2 in this commit: https://github.com/michael/github/commit/849e0588978519c80e01ee9704be515956a06d7c

Unfortunately, the next point release for github-api was 1.0.0 and it contains a couple of breaking changes where some functions were renamed and this breaks other parts of the JKAN code. So there is no point release of github-api on npm that works completely 😬

Two possible ways forward:

  1. a quick and dirty fix would be to vendor the working bundle of github-api from that commit
  2. a cleaner, but work-required fix would be to update to a newer version of github-api and chase down all of those breaking changes
wilsaj commented 8 years ago

Also, I can volunteer to work a PR for option 2 if you're up for reviewing it

timwis commented 8 years ago

Hey @wilsaj, thanks a lot for reporting the issue, and sorry you've experienced it. Your diagnosis was spot on and saved me a lot of time! I've just pushed a hotfix at #125 that's similar to your first suggestion but not quite vendoring (assuming by vendoring you mean including the code rather than a dependency) - let me know what you think and I'll merge it for now. Your second suggestion would certainly be ideal, and I'd love your help! Thanks a lot for taking the time.

Also, oddly, I don't get a build error with webpack :-/ was hoping to have travis run npm run build to catch this in the future.

wilsaj commented 8 years ago

Awesome! That hotfix totally works - I had no idea you could use specific github commits as package dependencies. I'll still work on updating github-api versions.

Also, oddly, I don't get a build error with webpack :-/ was hoping to have travis run npm run build to catch this in the future.

I used confusing language (sorry!). I didn't mean to indicate a build error, but that is definitely how it reads. The npm run build has no problem successfully building bundles, but the bundles would be built with this fundamental runtime bug (which manifests as a silently caught error and everyone is denied collaborator status even if they do have write permission). So there wasn't an easy way for someone else to build a usable JS bundle that could replace the one that is in scripts/dist/bundle.js.

wilsaj commented 8 years ago

Closing because this is taken care of now.

:+1: - thanks for all the work you put into JKAN!

timwis commented 8 years ago

I'm glad it's been useful! Just saw the open-austin fork - that's awesome! Feel free to reach out if there's anything I can do to help, and keep up the good work!