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

feat: options.id can be set to Client ID #98

Merged
merged 5 commits into from
May 2, 2024
Merged

feat: options.id can be set to Client ID #98

merged 5 commits into from
May 2, 2024

Conversation

gr2m
Copy link
Owner

@gr2m gr2m commented May 2, 2024

This pull request updates the handling and documentation of the id argument to accept both a number (App ID) or a string (Client ID), enhancing flexibility in accordance with GitHub's recommendations.

  • Updates type definitions: Modifies index.d.ts and internals.d.ts to allow the id parameter and appId result to be either a number or a string.
  • Revises documentation: Adjusts README.md to clarify that options.id can be set to either the App ID or Client ID, and recommends using the Client ID for github.com and GHES 3.14+. The resulting appId can also be a string accordingly.
  • Expands test coverage: Adds a new test case in index.test-d.ts and test/node.test.js to verify functionality when id is set to a string.

For more details, open the Copilot Workspace session.

index.test-d.ts Outdated Show resolved Hide resolved
Copy link
Owner Author

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

To avoid friction for downstream users, I think would make sense to implement the appId type of the result to be inferred by the id type passed to the function

Update: done ✅

@gr2m gr2m marked this pull request as draft May 2, 2024 21:09
@gr2m gr2m marked this pull request as ready for review May 2, 2024 21:18
@gr2m gr2m changed the title Update 'id' argument type to accept number or string feat: options.id can be set to Client ID May 2, 2024
@gr2m gr2m changed the base branch from main to beta May 2, 2024 21:23
@gr2m gr2m merged commit dc462fa into beta May 2, 2024
3 checks passed
@gr2m gr2m deleted the update-id-type branch May 2, 2024 21:23
Copy link

github-actions bot commented May 2, 2024

🎉 This PR is included in version 2.2.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant