Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rename forc doc --manifest-path to --path #6797

Merged
merged 8 commits into from
Jan 9, 2025

Conversation

amiremohamadi
Copy link
Contributor

Description

Closes #5789

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@amiremohamadi amiremohamadi requested a review from a team as a code owner December 20, 2024 09:27
@fuel-cla-bot
Copy link

fuel-cla-bot bot commented Dec 20, 2024

Thanks for the contribution! Before we can merge this, we need @amiremohamadi to sign the Fuel Labs Contributor License Agreement.

@zees-dev
Copy link
Contributor

There are still references to the manifest-path in the github workflow files; these will also need to be updated as part of the PR.

@amiremohamadi amiremohamadi requested a review from a team as a code owner December 24, 2024 02:58
Copy link
Member

@ironcev ironcev left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 😄 ❤️

@ironcev ironcev added the forc-doc Everything related to the `forc doc` command plugin. label Dec 30, 2024
Copy link
Member

@ironcev ironcev left a comment

Choose a reason for hiding this comment

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

@bitzoic We use to mark the breaking changes in the forc CLI with the breaking_change label and also to explain the change in the PR for devs who will eventually need to migrate their, e.g. CI scripts.

I would expect the same here. If devs, e.g., generate docs as a CI step this could break their workflows.

UPDATE: @amiremohamadi found a good solution for avoiding breaking change altogether, by adding the old manifest-path as an alias for the new path.

Copy link
Member

@bitzoic bitzoic left a comment

Choose a reason for hiding this comment

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

Thank you!

@JoshuaBatty JoshuaBatty merged commit bb855e8 into FuelLabs:master Jan 9, 2025
39 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cla:signed forc-doc Everything related to the `forc doc` command plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change forc doc --manifest-path to forc doc --path
6 participants