octokit / discussions

discussions and planning for Octokit clients
7 stars 5 forks source link

Transition plan for node-github #7

Closed zeke closed 7 years ago

zeke commented 7 years ago

@bkeepers, @kytrinyx, @gr2m, and I are working with the maintainers of the node-github module to make it an official octokit. Let's outline a plan for finding a new home for the project to thrive.

zeke commented 7 years ago

Updates:

Our current plan is to develop actively on the new @octokit fork, because that gives us control over CI and we don't have to keep bugging Mike about stuff like this. Once we feel ready to make the move. We can transfer mikedeboer/node-github to octokit/node-github so 301s will happen and there won't be any confusion about the new canonical home of the repo.

Regarding the eventual name of the repo and the npm module, my vote would be to leave them as is: node-github and github, respectively.

@gr2m and I will be working on getting a test suite in place tomorrow. If all goes well, I think we should be able to merge all the open PRs and cut a new major release either tomorrow or next week. 🤞

zeke commented 7 years ago

Oh yeah I also set up octokit/node-github#master as a protected branch so we can't force-push to it. Opted for minimum protection for now, but we might want to bump up the protection a bit since we have several non-GitHub folks in this octokit org.

screen shot 2017-08-17 at 12 12 30 pm

gr2m commented 7 years ago

Moving forward, I would suggest the following

move from mikedeboer/node-github to octokit/octokit.js

  1. rename repository to octokit.js. My understanding is that all SDKs shall be called octokit. And the library should work in all JavaScript environments, I would prefer octokit.js over say node-octokit
  2. We can continue publish the github package for now.
  3. Someone from GitHub should reach out to Phil to ask if we can take over the octokit package name. It hasn’t been update in 3 years, so we should be fine by just publishing a 1.0 which entirely breaks the previous APIs
  4. Deprecate github on npm with a message that the package has been renamed to octokit
  5. For the redirect from mikedeboer.github.io/node-github to octokit.github.io/octokit.js, we could ask Mike to create a repository mikedeboer.github.io which would have a single file node-github/index.html which would redirect to https://octokit.github.io/octokit.js with a meta tag. That way both the gh-pages and the repo redirects will work (all credit for that idea to @bkeepers đź‘Ť)

After the move

  1. setup auto-releasing with semantic-release
  2. setup Greenkeeper to keep dependencies up-to-date
  3. Add standard for linting I don’t want to do it before we take over node-github because it will change a lot of lines and cause lots of conflicts if other pull-requests are merged beforehand

Not blocked by the move

I can work on these things until the move is all figured out

  1. review open pull-requests and issues. I would suggest to create pull-request against the original repository as the fork will be deleted eventually and all pull-requests on it will be lost. Thoughts?
  2. make node-github compatible for the browser
bkeepers commented 7 years ago

That all sounds great to me.

Someone from GitHub should reach out to Phil to ask if we can take over the octokit package name.

Done.

zeke commented 7 years ago

[octokit] hasn’t been updated in 3 years, so we should be fine by just publishing a 1.0 which entirely breaks the previous APIs

libraries.io only shows a handful of dependent GitHub repos and npm packages, but the package is still downloaded over twenty times a day, on average. I wonder what that's about...

zeke commented 7 years ago

cc @philschatz đź‘‹

gr2m commented 7 years ago

here some updates from me

Repository

I prefer https://github.com/oktokit/octokit.js over https://github.com/oktokit/javascript, because "octokit.js" is what people will refer to it out here, and probably google for it. I definitely prefer https://github.com/oktokit/javascript over https://github.com/oktokit/js for clarity and SEO. "octokit.js" would also follow the conventions of "octokit.rb", "octokit.objc", "octokit.net".

Separate packages

I would setup the repository as a monorepo, which means multiple npm packages will be published from the same repository. There will be one octokit package which will be "all batteries included". But especially for usage in browsers, it makes sense to offer only the subset of functionality as separate packages. Another reason is that some functionality will be useful to other library/app developers. For example, a @octokit/preview-headers would be great to be reusable by others, too. It would only contain the headers which need to be set for the respective URLs, similar to https://github.com/philschatz/octokat.js/blob/master/src/grammar/preview-headers.coffee.

I like PouchDB’s monorepo setup: https://github.com/pouchdb/pouchdb. The overhead is minimal, and all packages are always released with the same version, following the conventions described at https://github.com/boennemann/alle. PouchDB has simple APIs for custom builds: https://pouchdb.com/custom.html

But before going in that direction, I’d setup a way to measure the built file sizes of the library to use in the browser and see if modularisation really has a significant impact or not.

Browser support

Making an octokit.js library run in both Node & browser is pretty straight forward. But with browser support comes the requirements for small bundle sizes. custom builds is one way to address it. Another one is to use browsers native APIs as much as possible and polyfill gaps in Node where a bigger file size is less of a problem.

I would recommend to utilise the new fetch API for requests. There is a drop-in library for node as well as a GitHub’s own polyfill for browsers which do not support fetch. This will significantly reduce the file size of octokit.js. Mikeal’s "spiritual successor to request" is a good showcase of that approach: https://github.com/mikeal/r2. I’ll evaluate r2 as the request library for octokit.js

Testing

I’ve created a separate project to creates fixtures, which automates keeping them up-to-date with GitHub API changes and exports a language agnostic mock-server binary. See my comment at https://github.com/octokit/discussions/issues/6#issuecomment-327614345

What’s next

It would be really helpful if we could finish the move from mikedeboer/node-github soon, or we could simply create an entire new repository octokit/octokit.js as lots of things will change anyway. But having one repository moving forward would make my life much easier maintaining the project 🙏

bkeepers commented 7 years ago

đź‘Ť that all sounds great.

It would be really helpful if we could finish the move from mikedeboer/node-github soon

Just to make sure I understand what's been proposed, we'll transfer it to the @octokit org, rename the repo to octokit.js, but continue publishing it as the github module for now?

Once the repo is transferred to octokit, we'll need @mikedeboer to push a version of this repo to his account.

gr2m commented 7 years ago

Just to make sure I understand what's been proposed, we'll transfer it to the @octokit org, rename the repo to octokit.js, but continue publishing it as the github module for now?

Yes, I would suggest to do that, until we addressed the currently open issues / PRs that are within the scope of the github module.

Once we work on the browser compatibility and replace the way it does requests internally, I would rename it to octokit and publish as 1.0.0, which is breaking version, so it will be okay with the currently released octokit package

zeke commented 7 years ago

This all sounds good to me đź‘Ť

philschatz commented 7 years ago

I am not sure where to post this but here are notes from a productive chat with @gr2m .

gr2m commented 7 years ago

Here some more notes based on my call with Phil and others

octokit.js

Typescript

I don’t work with typescript myself. I’m not sure if this is a hard requirement for octokit.js, but I at least want to research what a typescript as part of the library would imply.

Graph QL

I want to explore if it makes sense to use the GraphQL API under the hood, even for straight forward requests like octokit.repo.get({owner, user}). We could make allow to always pass an optional fields option with an array of properties that the method shall return. With that we could optimise the underlying request using Graph QL.

Fixtures

From phils comment

keeping ETag information in the fixtures has been super-helpful since some tests do: POST, GET, PUT, GET, DELETE, GET and you want different responses from the server

nock has the concept of mocks being used sequentially, so there are no etags necessary. But it’s still a smart idea, as Phil pointed out in our call, it would let us run tests in parallel against the mock server.

I’m eager to talk to the people working on the other Octokit SDKs to discuss how we could make octokit-fixtures usable for all.

Plugins

I think a solid plugin architecture will allow us to satisfy both: a low-level, small bundle sized core client and then utilise plugins to implement higher level APIs like "create pull request" similar to the experience on GitHub.com, where a fork is created automatically, I can assign people and set labels, all in one request.

Especially in JavaScript land, small modules is in the eco systems DNA. I would like to embrace it and try to put as many things as possible into plugins. These plugins can be officially supported and documented, just like the main octokit package. But it will still be easier to experiment with them that way. Some of them will probably find their way into the core client eventually, but it’s always easier to add a feature than to remove it.

Preview Headers

New APIs that require a preview header are currently built into both octokat.js and node-github. These APIs are by definition prone to change and I suggest we reflect that in the official octikit.js. I would suggest to move all preview APIs into plugins, make these plugins officially supported, too. That way when things break, we can easily release a new breaking version of the plugin without the main SDK to be affected

Debugging / Validation

I would not add it to octikit.js core, but would implement debugging / request validation in a separate plugin, too. The trick will be to expose low level extension points, similar to axios’ interceptors. A validation plugin could utilise a request interceptor / hook could validate the passed arguments based on the requested URL. A debugging / logging plugin could utilise all available hooks to log out helpful information based on the app’s needs.

zeke commented 7 years ago

utilise plugins to implement higher level APIs like "create pull request"

❤️

gr2m commented 7 years ago

@zeke Any update on the remaining two steps to finish the transition? Are there any blockers?

I went through the issues and PRs. I’ve added a new label pinned for issues that should no be closed. For the rest I’d suggest to enbale https://github.com/probot/stale now. I can’t do that myself, I don’t have the required authorization

zeke commented 7 years ago

Any update on the remaining two steps to finish the transition?

@bkeepers you set up an example GitHub pages redirect, didn't you? Are you down to send Mike some instructions on how to set it up? Once that's in place, we can transfer the repo.

bkeepers commented 7 years ago

I want to explore if it makes sense to use the GraphQL API under the hood, even for straight forward requests…

This is something we should explore eventually, but note that GraphQL is not currently supported with GitHub Apps and does not yet have feature parity with the REST API. Those are both high priorities for GitHub's platform team, but I think it'll be a while.

@bkeepers you set up an example GitHub pages redirect, didn't you? Are you down to send Mike some instructions on how to set it up? Once that's in place, we can transfer the repo.

Yep. Should we open an issue on the node-github repository to communicate that the transition is happening?

gr2m commented 7 years ago

Yes an issue would be good. There is no rush, I’m currently working on the test suite so we don’t break things as we iterate on the github package, but if you could create an issue, that’d be great. I think it would be helpful if the issue would be created by you or another GitHub employee

zeke commented 7 years ago

Issue sounds good. I would also email Mike and send him the issue URL, otherwise he probably will not see it.

bkeepers commented 7 years ago

@mikedeboer, I've added you as an admin to the @octokit org and we're ready to make the transfer! Here's what is needed:

  1. Accept the invitation to the @octokit org
  2. Go to Settings and transfer the repository to @octokit
  3. Create a new repository at mikedeboer/mikedeboer.github.io and push a clone of this repository, which will redirect https://mikedeboer.github.io/node-github/ to it's new home at https://octokit.github.io/node-github/

We'll take care of the rest from there, including communicating to existing users and contributors about the change.

Thanks!

mikedeboer commented 7 years ago

Done.

machour commented 7 years ago

Congratulations @mikedeboer 🙌

Don't forget to make this new octokit react-native compatible. Here's an ugly commit on a fork achieving it for reference: https://github.com/machour/node-github/commit/1ef08ecc6b537f1d39dee84f4204358a2fb88f5a

gr2m commented 7 years ago

Thanks y’all :)

gr2m commented 7 years ago

@machour can you please create an issue on https://github.com/octokit/node-github ?

machour commented 7 years ago

Bumped #273 on that repo, thanks!

zeke commented 7 years ago

🚀 Thanks everyone (and especially @gr2m) for making this happen.