Skip to content

feat: add warning for package in a workspace not listed as member #7123

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

khayss
Copy link

@khayss khayss commented Apr 27, 2025

Description

This PR closes #4451

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.

@fuel-cla-bot
Copy link

fuel-cla-bot bot commented Apr 27, 2025

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

@khayss
Copy link
Author

khayss commented Apr 27, 2025

@IGI-111
I'm making this draft PR to discuss some details about implementing this feature.

Since this is inspired by Cargo design, I have studied how Cargo achieves this feature, and I have the following questions that will be helpful in completing this feature.

  1. Is there a current functionality to locate the workspace manifest from a sub-directory?
  2. Is there any recommendation on where this feature should be implemented? i.e., location or module?

Anticipating your response 🫡

@khayss
Copy link
Author

khayss commented Apr 29, 2025

@IGI-111 any update?

Copy link
Member

@kayagokalp kayagokalp 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 PR, we seem to have lots of cargo references here in the code, I haven't checked the overall code but this seems to me either fully LLM generated or copy pasted from cargo, which probably won't be working if we push it like this :) I can help out if you need pointers but this PR as it is not seem to be going in a correct direction.

First of all we need to be re-using availble stuff from forc-pkg and forc-util, in forc side you need to check if we are in a workspace, in new and init commands, if so we should add ourselves (the package we are processing) to the workspace.

If we invoked forc build, we should again check if we are in a workspace, if so we should warn and say that this appears to be in a workspace but not listed as a member, this check probably needs to happen on forc-pkg side.

To summarize we need to ensure we can do this check on forc-pkg side, and invoke this check for new, init and build. Simply consume it differently on new, init and build scenerios

@khayss
Copy link
Author

khayss commented May 1, 2025

Thanks for the update @kayagokalp
I reused some codes from Cargo and restructured where necessary. I'll have a look at the requested changes and implement them.

@khayss
Copy link
Author

khayss commented May 1, 2025

Also, for my question, I'd like to know if there's a logic to locate a workspace manifest from a sub-directory?

@kayagokalp

@khayss
Copy link
Author

khayss commented May 3, 2025

Hello @kayagokalp
Pls check and review

@khayss
Copy link
Author

khayss commented May 3, 2025

I understand where the feature is to be implemented. For now, the most important thing is having a way to find the workspace manifest from the current directory. This current draft aims to solve that, as there's no such functionality yet. If this is reviewed, and it's cool, I can proceed to finish up the feature

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.

Warn users if they create a package in a workspace without listing them as a member
2 participants