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

Refactor forc init + add forc new #1669

Closed
wants to merge 13 commits into from
Closed

Conversation

ra0x3
Copy link
Contributor

@ra0x3 ra0x3 commented May 25, 2022

Description

  • This PR changes forc init to forc new, and updates forc init functionality to be more similar to cargo init

Testing steps

  • Try pulling down the PR and creating a forc init project with combinations of the different flags

Read the Docs:

Join the Community:

Report Bugs:

2
�[0m�[32mSuccessfully created contract: sway
�[0m
To compile, use `forc build`, and to run tests use `forc test`

----

Read the Docs:
- Sway Book: https://fuellabs.github.io/sway/latest
- Rust SDK Book: https://fuellabs.github.io/fuels-rs/latest
- TypeScript SDK: https://github.com/FuelLabs/fuels-ts

Join the Community:
- Follow us @swaylang: https://twitter.com/SwayLang
- Ask questions in dev-chat on Discord: https://discord.com/invite/xfpK4Pe

Report Bugs:
- Sway Issues: https://github.com/FuelLabs/sway/issues/new add
@ra0x3 ra0x3 self-assigned this May 25, 2022
@ra0x3 ra0x3 marked this pull request as draft May 25, 2022 17:04
@ra0x3 ra0x3 added forc breaking May cause existing user code to break. Requires a minor or major release. labels May 25, 2022
docs/src/SUMMARY.md Outdated Show resolved Hide resolved
@ra0x3 ra0x3 force-pushed the rashad/1638-forc-init-forc-new branch from f742e9e to 4a1cdac Compare May 26, 2022 17:28
@ra0x3 ra0x3 marked this pull request as ready for review May 26, 2022 21:25
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Great to see this happening!


I think we can remove a lot of duplicated code/logic between forc new and forc init.

We can think of

forc new <path>

very roughly as a tiny short-hand wrapper around:

mkdir -p <path>
forc init --path <path>

Let's refactor the forc new logic here to take advantage of the forc init functionality to make this easier to maintain and keep behaviour consistent.

forc/src/cli/commands/init.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/init.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/init.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/init.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/new.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/new.rs Show resolved Hide resolved
forc/src/cli/commands/new.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_new.rs Outdated Show resolved Hide resolved
@adlerjohn adlerjohn marked this pull request as draft May 27, 2022 12:51
@adlerjohn
Copy link
Contributor

Converting to draft both because will a few comments to address, and so as not to merge it during the hackathon by accident since it's a breaking change.

docs/src/introduction/standard_library.md Outdated Show resolved Hide resolved
forc-util/src/restricted.rs Outdated Show resolved Hide resolved
forc/src/cli/commands/init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
@ra0x3 ra0x3 requested a review from mitchmindtree May 31, 2022 16:54
@adlerjohn adlerjohn marked this pull request as ready for review June 1, 2022 01:04
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Just a couple more small things, then I'll test locally and we should be good to go!

forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
forc/src/ops/forc_init.rs Outdated Show resolved Hide resolved
JoshuaBatty
JoshuaBatty previously approved these changes Jun 2, 2022
Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

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

Nice work. Pulled down and tested locally. All looks good.

Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Just opened #1841 to cut down on some of the duped logic.

forc-util/src/lib.rs Outdated Show resolved Hide resolved
@ra0x3 ra0x3 force-pushed the rashad/1638-forc-init-forc-new branch from d1cbb82 to b80ffa1 Compare June 15, 2022 23:55
@ra0x3 ra0x3 marked this pull request as draft June 29, 2022 20:13
@ra0x3 ra0x3 closed this Jun 29, 2022
@ra0x3 ra0x3 deleted the rashad/1638-forc-init-forc-new branch June 29, 2022 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking May cause existing user code to break. Requires a minor or major release. forc
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

forc new and forc init should match cargo new and cargo init behaviour respectively
4 participants