Skip to content

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.

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
@gr2m gr2m deleted the update-id-type branch May 2, 2024 21:23
@github-actions
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.

2 participants