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

[Chore] [Blocked]: "github" dependency renamed to "@octokit/rest" #482

Closed
fbartho opened this issue Jan 17, 2018 · 12 comments
Closed

[Chore] [Blocked]: "github" dependency renamed to "@octokit/rest" #482

fbartho opened this issue Jan 17, 2018 · 12 comments

Comments

@fbartho
Copy link
Member

fbartho commented Jan 17, 2018

I noticed in the terminal on package install the following warning

$ yarn install
…
warning danger > github@13.1.0: 'github' has been renamed to '@octokit/rest' (https://git.io/vNB11)
…

Looking at the release page: https://github.com/octokit/rest.js/releases/tag/v14.0.0 I see:

there are no functional changes in the public APIs compared to github@13.

I'll submit a PR shortly after find-replacing everything & running the tests.

@orta
Copy link
Member

orta commented Jan 17, 2018

Wow, that was quick, that release was 4 hours ago :D

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

@orta haha, I got excited to help when I saw the warning :P

Maybe too excited… I unfortunately got stuck when trying to push. The typescript compiler is grumpy:

source/dsl/GitHubDSL.ts(2,25): error TS7016: Could not find a declaration file for module '@octokit/rest'. '/Users/fbarthelemy/Code/danger-js/node_modules/@octokit/rest/index.js' implicitly has an 'any' type.
  Try `npm install @types/@octokit/rest` if it exists or add a new declaration (.d.ts) file containing `declare module '@octokit/rest';`

2 import * as GitHub from "@octokit/rest"
// ...(repeat a bunch)

When I add

declare module "@octokit/rest"

to ambient.d.ts

Then I get:

source/dsl/GitHubDSL.ts(2,13): error TS6133: 'GitHub' is declared but its value is never read.

2 import * as GitHub from "@octokit/rest"
// ...(repeat a bunch)

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

(Note: this compiler error only occurs on push, the tests all pass with the find-replace)

@orta
Copy link
Member

orta commented Jan 18, 2018

Ah yeah, definitely typed needs to be updated with the new module.

@orta
Copy link
Member

orta commented Jan 18, 2018

Or not, will have a look

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

Looking at: https://www.npmjs.com/package/@types/github -- no types should be needed.

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

Should I force the branch up so you can start from passing tests with all the find-replaces complete?

@orta
Copy link
Member

orta commented Jan 18, 2018

Perfect, yeah

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

@orta
Copy link
Member

orta commented Jan 18, 2018

Yeah, they're accidentally not shipping the types

screen shot 2018-01-17 at 7 11 55 pm

Should be a index.d.ts.

@fbartho
Copy link
Member Author

fbartho commented Jan 18, 2018

Nice catch. I'm new to typescript & module namespaces, so I didn't know what I should be looking for. I can file a bug with them.

@orta
Copy link
Member

orta commented Jan 18, 2018

No worries, it's definitely something you learn with time 👍

@fbartho fbartho changed the title [Chore]: "github" dependency renamed to "@octokit/rest" [Chore] [Blocked]: "github" dependency renamed to "@octokit/rest" Jan 18, 2018
@orta orta closed this as completed in 016e97a Jan 20, 2018
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

No branches or pull requests

2 participants