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

Remove unnecessary trailing / in prepare-docs.sh #5323

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

0xwitty
Copy link
Contributor

@0xwitty 0xwitty commented Nov 26, 2024

The script contained a small but impactful issue in this line:

name="${f/#"$examples_source_dir/"/}"

The corrected version is:

name="${f/#"$examples_source_dir"/}"

Issue Explanation

The extra / after "$examples_source_dir" in the bash string substitution caused incorrect path handling. This could result in errors or malformed paths when processing examples, particularly when attempting to adjust imports and structure.

Why It Matters

  • Functionality: This fix ensures the examples are correctly copied and their imports are properly adjusted, which is critical for generating accurate documentation.
  • Maintainability: Removing this unnecessary character improves script readability and reduces potential confusion for future contributors.
  • Compatibility: The fix ensures the script behaves reliably across different environments and path configurations.

This small change prevents potential issues during the documentation generation process and guarantees smoother execution.

PR Checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

Copy link

changeset-bot bot commented Nov 26, 2024

⚠️ No Changeset found

Latest commit: b79052b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Amxx Amxx added ignore-changeset documentation Inline comments, guides, and examples. CI labels Nov 27, 2024
@Amxx Amxx requested a review from ernestognw December 19, 2024 10:20
@Amxx
Copy link
Collaborator

Amxx commented Dec 19, 2024

@ernestognw you worked a lot on docs. Can you please check this ? If you approve, feel free to merge with my second approval.

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

Both the old and new versions produce the same output so they're equivalent.

May be relevant since examples_source_dir does not have a trailing slash (/), but it seems that this line is treating / as a literal instead of a character.

@ernestognw ernestognw changed the title Fixed a small but significant issue in prepare-docs.sh Remove unnecessary trailing / in prepare-docs.sh Dec 20, 2024
@ernestognw ernestognw merged commit ba8b5cf into OpenZeppelin:master Dec 20, 2024
17 checks passed
Copy link

gitpoap-bot bot commented Dec 20, 2024

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2024 OpenZeppelin Contracts Contributor:

GitPOAP: 2024 OpenZeppelin Contracts Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI documentation Inline comments, guides, and examples. ignore-changeset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants