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

meta: forge init improvements #1132

Closed
3 of 7 tasks
mds1 opened this issue Mar 30, 2022 · 7 comments
Closed
3 of 7 tasks

meta: forge init improvements #1132

mds1 opened this issue Mar 30, 2022 · 7 comments
Labels
C-forge Command: forge Cmd-forge-pm Command: forge install/update/remove D-chore Difficulty: chore good first issue Good for newcomers P-low Priority: low T-feature Type: feature

Comments

@mds1
Copy link
Collaborator

mds1 commented Mar 30, 2022

Component

Forge

Describe the feature you would like

Here are some small changes that I think would improve the quality of forge init:

I can see an argument against (3) since not everyone uses github. But it's very rare I come across gitlab or bitbucket repos, and it's easy to remove the workflow, so I think this tradeoff is worth it

Additional context

No response

@mds1 mds1 added the T-feature Type: feature label Mar 30, 2022
@dmfxyz
Copy link
Contributor

dmfxyz commented Mar 30, 2022

For (3), what dispatch triggers should the workflow have? I agree it would be good to include, but I suggest that since it's being automatically added, only a workflow_dispatch should be included by default. Those running in private repos or with self-hosted runners may be sensitive to gha usage.

@DanielVF
Copy link

I'm all for moving the test directory up a level.

@onbjerg onbjerg added good first issue Good for newcomers C-forge Command: forge P-low Priority: low D-chore Difficulty: chore Cmd-forge-pm Command: forge install/update/remove labels Mar 30, 2022
@onbjerg
Copy link
Member

onbjerg commented Mar 30, 2022

I agree, moving the tests to root makes sense and it wouldn't break anything since we would still run tests in src as we do now (pinging @mattsse for confirm on that)

@mds1
Copy link
Collaborator Author

mds1 commented Apr 16, 2022

Just noting here that "Verifies all non-test contracts are below the size limit" can now be implemented

@sambacha
Copy link
Contributor

Is it possible to first have an empty commit then the actual forge init commit when creating a repo?

Maybe also a flag for --makefile/--justfile or even --template-url

@onbjerg
Copy link
Member

onbjerg commented Apr 18, 2022

Yes, forge init --no-commit. Please refer to the docs: https://book.getfoundry.sh/reference/forge/forge-init.html#vcs-options

I really don't think we should start cluttering init with a bunch of --<config file> flags which is also why I was initially against --vscode

@onbjerg onbjerg added this to the v1.0.0 milestone Jul 1, 2022
@onbjerg onbjerg removed this from the v1.0.0 milestone Aug 23, 2022
@mds1
Copy link
Collaborator Author

mds1 commented Sep 20, 2023

Going to close this issue. I think the existing simple forge init is sufficient, and there are now many third party templates that each have their own defaults and opinions. Some examples:

@mds1 mds1 closed this as completed Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-pm Command: forge install/update/remove D-chore Difficulty: chore good first issue Good for newcomers P-low Priority: low T-feature Type: feature
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants