Skip to content

Conversation

mahabaleshwars
Copy link
Contributor

@mahabaleshwars mahabaleshwars commented Jun 6, 2025

Description:

  • Added support for using a github_token from the environment to make authenticated requests to GitHub, which helps avoid API rate limits when downloading GraalVM.
  • Refactored the codebase for improved readability and maintainability.

Related issue:
#481

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@mahabaleshwars mahabaleshwars self-assigned this Jun 6, 2025
@robstoll
Copy link

robstoll commented Jun 7, 2025

out of curiosity, I also hit API Rate Limit issues when using jetbrains does your fix address this as well?

@mahabaleshwars mahabaleshwars marked this pull request as ready for review June 11, 2025 09:40
@Copilot Copilot AI review requested due to automatic review settings June 11, 2025 09:40
@mahabaleshwars mahabaleshwars requested a review from a team as a code owner June 11, 2025 09:40
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for GitHub tokens to address API rate limits and refactors code for improved readability and maintainability.

  • Introduces an optional token parameter to the GitHub HTTP headers helper.
  • Refactors URL construction and error handling in the GraalVM installer.
  • Updates associated tests with better grouping and improved mocking.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File Description
src/util.ts Added an optional token parameter to getGitHubHttpHeaders to support GitHub token usage.
src/distributions/graalvm/installer.ts Refactored file URL construction, error handling, and platform logic for the GraalVM installer.
tests/distributors/graalvm-installer.test.ts Improved test grouping, added helpers for mocking HTTP responses, and refined architecture mapping tests.
Comments suppressed due to low confidence (1)

src/util.ts:186

  • Consider updating the function's documentation or inline comments to clearly describe the new optional 'token' parameter and its behavior.
export function getGitHubHttpHeaders(token?: string): OutgoingHttpHeaders {

const osType = distribution.getPlatform();
if (osType === 'windows' && distroArch == 'aarch64') {
if (osType === 'windows' && distroArch === 'aarch64') {
return; // skip, aarch64 is not available for Windows
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Instead of returning early inside the test body to skip the unsupported Windows aarch64 case, consider using a conditional test skip (e.g. test.skip for that iteration) to make the intent clearer.

Suggested change
return; // skip, aarch64 is not available for Windows
test.skip('Skipping test: aarch64 is not available for Windows');

Copilot uses AI. Check for mistakes.

@mahabaleshwars mahabaleshwars changed the title Add GitHub Token Support and Refactor Code. Add GitHub Token Support for GraalVM and Refactor Code Jul 3, 2025
@mahabaleshwars mahabaleshwars added the feature request New feature or request to improve the current logic label Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request to improve the current logic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants