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

[discussion]: Put foundations on Octokit namespace #308

Open
oscard0m opened this issue Oct 9, 2020 · 0 comments
Open

[discussion]: Put foundations on Octokit namespace #308

oscard0m opened this issue Oct 9, 2020 · 0 comments
Labels
project Goes beyond a single bug fix or new feature. Help welcome by existing contributors. Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR typescript Relevant to TypeScript users only
Projects

Comments

@oscard0m
Copy link
Member

oscard0m commented Oct 9, 2020

Description

To start defining and discussing the foundations of Octokit namespace.

Original messages:


I gave this some more thought, and I think I'd actually prefer what express is doing by defining a global namespace, see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/express-serve-static-core/index.d.ts#L17-L25

The reason for that is that it is possible that these type definitions will move to other packages in future, e.g. an @octokit/events package or @octokit/types. My understanding is that this would require changes to the declare "@octokit/webhooks" blocks, while the global namespace approach would always work

declare namespace Octokit {
    export interface Events {
        "my_custom_event": MyCustomEventPayload
    }
}

Originally posted by @gr2m in #250 (comment)


In Typescript interfaces merging is allowed. I'll have to test how it'll pick up as the interfaces would be in different files. We might need to wrap the interface in a module.

_Originally posted by @ankeetmaini in #250 (comment)


I want to give it more thought, a global Octokit namespace would affect all Octokit libraries. We can start experimenting now, I just don't want to rush ahead and build it, to avoid disruptive changes once we put it all together with the types from the REST API endpoints, etc

_Originally posted by @gr2m in #250 (comment)

@ghost ghost added this to Inbox in JS Oct 9, 2020
@oscard0m oscard0m changed the title [discussion]: Put fundations on Octokit namespace [discussion]: Put foundations on Octokit namespace Oct 9, 2020
@oscard0m oscard0m added Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR typescript Relevant to TypeScript users only labels Oct 9, 2020
@ghost ghost moved this from Inbox to Maintenance in JS Oct 9, 2020
@gr2m gr2m added the project Goes beyond a single bug fix or new feature. Help welcome by existing contributors. label Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project Goes beyond a single bug fix or new feature. Help welcome by existing contributors. Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR typescript Relevant to TypeScript users only
Projects
No open projects
JS
  
Maintenance
Development

No branches or pull requests

2 participants