Skip to content

chore: improve code style#512

Merged
ngundotra merged 12 commits intosolana-foundation:masterfrom
rogaldh:chore/improve-code-style
May 14, 2025
Merged

chore: improve code style#512
ngundotra merged 12 commits intosolana-foundation:masterfrom
rogaldh:chore/improve-code-style

Conversation

@rogaldh
Copy link
Copy Markdown
Contributor

@rogaldh rogaldh commented Mar 25, 2025

Description

This PR improves configuration for prettier. The pattern to check for is changed also. This allows to cover more files than it was before.

Type of change

  • New feature
  • Other (please describe): improve configuration, adjust workflow

Testing

pnpm format:ci

Checklist

  • My code follows the project's style guidelines
  • I have added tests that prove my fix/feature works
  • All tests pass locally and in CI
  • CI/CD checks pass

Important

Improves code style by configuring Prettier, updating CI workflow, and reformatting codebase.

  • Prettier Configuration:
    • Added .prettierrc.cjs with custom configuration extending @solana/prettier-config-solana.
    • Added .prettierignore to exclude certain directories and files from formatting.
  • CI Workflow:
    • Updated ci.yaml to include a Code-style step running pnpm format:ci.
  • Code Formatting:
    • Reformatted code in app/__tests__/mock-stubs.ts, app/__tests__/mocks.ts, and app/api/domain-info/[domain]/route.ts to adhere to new Prettier rules.
    • Applied consistent formatting in app/api/metadata/proxy/__tests__/endpoint.test.ts and app/api/metadata/proxy/__tests__/fetch-resource.spec.ts.
    • Updated package.json to use .prettierrc.cjs for formatting.

This description was created by Ellipsis for a652be1. It will automatically update as commits are pushed.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 25, 2025

@rogaldh is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to a652be1 in 1 minute and 57 seconds

More details
  • Looked at 1326 lines of code in 28 files
  • Skipped 1 files when reviewing.
  • Skipped posting 35 drafted comments based on config settings.
1. .github/workflows/ci.yaml:38
  • Draft comment:
    Good addition of a 'Code-style' step. The workflow now checks formatting via 'pnpm format:ci', which improves code quality.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. .prettierrc.cjs:1
  • Draft comment:
    The prettier configuration is clear and leverages the shared Solana configuration nicely. No issues found.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. app/__tests__/mocks.ts:12
  • Draft comment:
    Improved formatting and consistent use of single quotes in imports and mocking functions is good.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. app/api/domain-info/[domain]/route.ts:15
  • Draft comment:
    The API route code style improvements (e.g., single quotes and formatting) enhance readability. The conditional call for getDomainInfo versus getANSDomainInfo is clear.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. app/api/metadata/proxy/__tests__/endpoint.test.ts:13
  • Draft comment:
    The test mocks for 'node-fetch' and 'dns' are well-structured, ensuring that endpoint behavior is properly simulated. This helps in reliable CI.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. package.json:11
  • Draft comment:
    The new 'format:ci' script now covers additional file patterns for proper code style checking. Also, note the override for 'node-fetch'. Ensure that using node-fetch v2 (override) versus dependency v3 is intentional.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. vite.config.mts:6
  • Draft comment:
    The Vite config improvements with clear alias definitions and pooling options for tests enhance project maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. test-setup.ts:7
  • Draft comment:
    The test setup polyfills, such as AbortSignal.timeout and extending Uint8Array instance checks, are clear and well-documented.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
9. app/utils/domain-info.ts:24
  • Draft comment:
    The domain info functions are straightforward. The use of try/catch around getNameOwner and getANSNameOwner helps in graceful error handling.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
10. app/utils/feature-gate/utils.ts:6
  • Draft comment:
    Using useMemo for feature info retrieval is a good practice to memoize values and enhance performance.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
11. app/utils/token-info.ts:89
  • Draft comment:
    The token info utilities combine on-chain and off-chain lookups effectively. Ensure that the merging logic in getFullTokenInfo prioritizes up-to-date info as expected.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
12. next.config.mjs:21
  • Draft comment:
    Redirect configuration is clear. Double-check that the alias mappings cover all required paths for legacy URLs.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
13. test-setup.ts:17
  • Draft comment:
    The extension of Uint8Array[Symbol.hasInstance] ensures compatibility with Buffer instances. This is a neat workaround addressing known issues.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
14. vite.config.mts:38
  • Draft comment:
    The poolOptions configuration for Vitest using Atomics enhances test performance in multi-threaded environments.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
15. .github/workflows/ci.yaml:38
  • Draft comment:
    Good addition of the 'Code-style' step using 'pnpm format:ci' to enforce code formatting in CI. This helps keep the codebase consistent.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, praising the addition of a code formatting step. It doesn't provide any actionable feedback or suggestions for improvement, nor does it point out any potential issues or areas for confirmation. According to the rules, purely informative comments should be removed.
16. .prettierrc.cjs:4
  • Draft comment:
    Extending the '@solana/prettier-config-solana' config and adding specific overrides is well organized and broadens file coverage.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply acknowledges a change without offering any critique or guidance.
17. app/api/metadata/proxy/feature/ip.ts:48
  • Draft comment:
    The check for localhost includes a condition for hostname equal to '0'. Please verify that handling '0' as a localhost is intentional since it's uncommon.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
18. app/api/metadata/proxy/feature/processors.ts:41
  • Draft comment:
    Handling a JSON parsing error by throwing errors[415] clearly signals a malformed JSON response. This is a sensible approach.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is purely informative, as it simply states that handling a JSON parsing error by throwing errors[415] is a clear signal of a malformed JSON response. It does not provide any actionable feedback or suggestions for improvement.
19. next.config.mjs:29
  • Draft comment:
    The redirects configuration is comprehensive. Make sure that the ordering of redirect rules (especially for aliases) works as expected.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that the ordering of redirect rules works as expected. It is not making a specific suggestion or pointing out a specific issue, which violates the rule against asking the author to ensure behavior is intended.
20. package.json:11
  • Draft comment:
    The updated prettier command and the 'format:ci' script are well aligned with the new configuration. Double-check that 'pnpm format:ci' correctly triggers the format check in CI.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to double-check the behavior of a command in CI, which violates the rule against asking the author to confirm or ensure behavior. It does not provide a specific code suggestion or ask for a specific test to be written.
21. app/api/metadata/proxy/__tests__/endpoint.test.ts:83
  • Draft comment:
    Typo: The comment 'fail on encoded incorrectly input' is confusing. Consider changing it to 'fail on incorrectly encoded input'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
22. app/api/metadata/proxy/__tests__/endpoint.test.ts:100
  • Draft comment:
    Typographical note: The header key 'Etag' is used. For consistency with common HTTP header conventions, consider using 'ETag' (with an uppercase 'T').
  • Reason this comment was not posted:
    Comment was on unchanged code.
23. app/api/metadata/proxy/__tests__/fetch-resource.spec.ts:59
  • Draft comment:
    Typo detected: In the error message on line 59, 'resour.ce' appears to be a typo. It should likely be 'resource'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
24. app/api/metadata/proxy/__tests__/ip.spec.ts:40
  • Draft comment:
    Typo: The phrase 'forinvalid' should be corrected to 'for invalid' to improve readability.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
25. app/api/metadata/proxy/feature/index.ts:51
  • Draft comment:
    Typographical suggestion: Consider revising the comment on handling non-Error exceptions to improve clarity. For example, you might change "handle any other error as general one and allow to see it at console" to "handle any other error as a general error and log it to the console".
  • Reason this comment was not posted:
    Comment was on unchanged code.
26. app/api/metadata/proxy/feature/index.ts:84
  • Draft comment:
    Typographical suggestion: The comment "otherwise we throw error as we getting unexpected content" could be improved for clarity. Consider changing it to "otherwise, throw an error because unexpected content was received".
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
27. app/api/metadata/proxy/feature/ip.ts:54
  • Draft comment:
    Typo detected in the comment on line 54: 'Check for IP address to be in private rande'. Please correct 'rande' to 'range'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
28. app/api/metadata/proxy/feature/processors.ts:29
  • Draft comment:
    Consider capitalizing 'json' to 'JSON' in the comment on line 29 for consistency and clarity, since JSON is an acronym.
  • Reason this comment was not posted:
    Comment was on unchanged code.
29. app/api/metadata/proxy/route.ts:43
  • Draft comment:
    Consider rephrasing the comment on line 43 for clarity. Perhaps change it to: "Check that the URI uses a supported protocol regardless of other validations." This helps in making the intent clearer.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
30. app/components/inspector/into-parsed-data.ts:29
  • Draft comment:
    Typographical error: In the function discriminatorToBuffer, the parameter is named 'discrimnator'. Consider renaming it to 'discriminator' for clarity and correctness.
  • Reason this comment was not posted:
    Comment was on unchanged code.
31. app/components/inspector/utils.ts:18
  • Draft comment:
    The comment on line 18 has an awkward phrasing. Consider rewording it to something like: 'dynamic means that lookups are taken not from staticAccountKeys' for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
32. app/features/feature-gate/index.ts:36
  • Draft comment:
    Line 36: Consider changing 'Can not fetch link:' to 'Cannot fetch link:' to align with standard English usage. This is a trivial typographical improvement.
  • Reason this comment was not posted:
    Comment was on unchanged code.
33. app/utils/__tests__/epoch-schedule.ts:71
  • Draft comment:
    Typo in test description: It states 'returns the first slot for an epoch before firstNormalEpoch', but it should say 'returns the last slot for an epoch before firstNormalEpoch'. Please update the description for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
34. app/utils/domain-info.ts:23
  • Draft comment:
    Typographical note: Consider changing 'returns non empty wallet string' to 'returns non-empty wallet string' for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
35. app/utils/domain-info.ts:47
  • Draft comment:
    Typographical note: Consider updating 'returns only non expired domains' to 'returns only non-expired domains' for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_dBZoEMGrwlzvwBY4


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rogaldh rogaldh force-pushed the chore/improve-code-style branch from a652be1 to 665e9dc Compare April 9, 2025 16:28
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2025 1:39pm

@ngundotra
Copy link
Copy Markdown
Contributor

ngundotra commented Apr 9, 2025

Seeing this in Vercel logs...

Warning: Due to "engines": { "node": "v20.19" } in your `package.json` file, the Node.js Version defined in your Project Settings ("18.x") will not apply, Node.js Version "20.x" will be used instead. Learn More: http://vercel.link/node-version
Restored build cache from previous deployment (3RTXRe4Y5PdkMDC5gUK2ioEcjXVG)
Running "vercel build"
Vercel CLI 41.5.0
Warning: Due to "engines": { "node": "v20.19" } in your `package.json` file, the Node.js Version defined in your Project Settings ("18.x") will not apply, Node.js Version "20.x" will be used instead. Learn More: http://vercel.link/node-version
Detected `pnpm-lock.yaml` 9 which may be generated by pnpm@9.x or pnpm@10.x
Using pnpm@9.x based on project creation date
To use pnpm@10.x, manually opt in using corepack (https://vercel.com/docs/deployments/configure-a-build#corepack)
Running "install" command: `pnpm install`...
Checking for package peerDependency overrides
 WARN  Unsupported engine: wanted: {"node":"v20.19"} (current: {"node":"v20.18.3","pnpm":"9.15.9"})
 ERR_PNPM_LOCKFILE_CONFIG_MISMATCH  Cannot proceed with the frozen installation. The current "pnpmfileChecksum" configuration doesn't match the value found in the lockfile
Update your lockfile using "pnpm install --no-frozen-lockfile"
Error: Command "pnpm install" exited with 1

@rogaldh
Copy link
Copy Markdown
Contributor Author

rogaldh commented Apr 17, 2025

Seeing this in Vercel logs...

Warning: Due to "engines": { "node": "v20.19" } in your `package.json` file, the Node.js Version defined in your Project Settings ("18.x") will not apply, Node.js Version "20.x" will be used instead. Learn More: http://vercel.link/node-version
Restored build cache from previous deployment (3RTXRe4Y5PdkMDC5gUK2ioEcjXVG)
Running "vercel build"
Vercel CLI 41.5.0
Warning: Due to "engines": { "node": "v20.19" } in your `package.json` file, the Node.js Version defined in your Project Settings ("18.x") will not apply, Node.js Version "20.x" will be used instead. Learn More: http://vercel.link/node-version
Detected `pnpm-lock.yaml` 9 which may be generated by pnpm@9.x or pnpm@10.x
Using pnpm@9.x based on project creation date
To use pnpm@10.x, manually opt in using corepack (https://vercel.com/docs/deployments/configure-a-build#corepack)
Running "install" command: `pnpm install`...
Checking for package peerDependency overrides
 WARN  Unsupported engine: wanted: {"node":"v20.19"} (current: {"node":"v20.18.3","pnpm":"9.15.9"})
 ERR_PNPM_LOCKFILE_CONFIG_MISMATCH  Cannot proceed with the frozen installation. The current "pnpmfileChecksum" configuration doesn't match the value found in the lockfile
Update your lockfile using "pnpm install --no-frozen-lockfile"
Error: Command "pnpm install" exited with 1

Yep. This should gone after rebasing upon the master. That's because we did use matrix for 18.x

chore: improve config for prettier

format existing code with new preset

format existing code with new preset

apply updated configuration for the project

chore: add ci-specific script to check code-style upon changes at CI

check prettier issue

chore: improve config for prettier

format existing code with new preset

format existing code with new preset

apply updated configuration for the project

chore: add ci-specific script to check code-style upon changes at CI

check prettier issue

fix
@rogaldh rogaldh force-pushed the chore/improve-code-style branch from 105df43 to a791fad Compare April 17, 2025 17:55
rogaldh added 4 commits May 12, 2025 17:20
chore: improve config for prettier

format existing code with new preset

format existing code with new preset

apply updated configuration for the project

chore: add ci-specific script to check code-style upon changes at CI

check prettier issue

chore: improve config for prettier

format existing code with new preset

format existing code with new preset

apply updated configuration for the project

chore: add ci-specific script to check code-style upon changes at CI

check prettier issue

fix
@rogaldh rogaldh force-pushed the chore/improve-code-style branch from 640355f to 09f7200 Compare May 12, 2025 18:40
@ngundotra
Copy link
Copy Markdown
Contributor

Thanks for this! This looks all good

@ngundotra ngundotra merged commit 6e0123a into solana-foundation:master May 14, 2025
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants