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

feat: make scaffolding cli package manager agnostic #316

Merged
merged 32 commits into from
Jun 28, 2024

Conversation

c12i
Copy link
Collaborator

@c12i c12i commented Jun 20, 2024

Closes #268

Depends on #313

@c12i c12i force-pushed the parameterize-package-managers branch from 60e0af4 to 0b35b7a Compare June 20, 2024 15:32
@c12i c12i added the ShouldBackport/0.3 This change should be backported to develop-0.3 label Jun 24, 2024
@c12i c12i changed the title feat: add package manager module feat: make scaffolding cli package manager agnostic Jun 24, 2024
@c12i c12i marked this pull request as ready for review June 26, 2024 08:36
@c12i c12i requested a review from matthme June 26, 2024 08:36
@c12i c12i force-pushed the parameterize-package-managers branch from 891843b to 00027f0 Compare June 26, 2024 11:05
@c12i c12i force-pushed the parameterize-package-managers branch from f39e248 to 30490bd Compare June 26, 2024 12:45
@c12i c12i requested a review from pdaoust June 26, 2024 13:05
@@ -1,6 +1,37 @@
# Holochain Scaffolding CLI

CLI to easily generate and edit holochain apps.
A command-line interface for creating and modifying a Holochain application (hApp).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pdaoust I'd like to know whether this doc should be in the developer reference rather than here in the scaffolding cli?

Copy link
Collaborator

Choose a reason for hiding this comment

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

😬 that is a very good question for which I have no answer. Ideally we'd have documentation 'wherever the dev finds themselves', so I would like to say 'yes to both places'. But that creates duplicated content. I wonder if we could write something that would inline it into the docs-pages build pipeline.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can publish this through Rust docs and then add a link to it in the developer guide, what do you think?

Copy link
Collaborator

@matthme matthme left a comment

Choose a reason for hiding this comment

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

Hmm...it seems to me that the --package-manager option should not be at the top level but instead be an option of the web-app and example subcommands as it does not seem to have any effect beyond the initial web-app scaffolding step.
And in case that makes sense to you then I think it would be quite nice to have an interactive selection step where one can choose the package manager explicitly, akin to how the UI framework can be chosen.

@c12i
Copy link
Collaborator Author

c12i commented Jun 27, 2024

Good idea, thought I would give it the same treatment as the --template flag, but you are right, the option is only relevant to hc-scaffold web-app and hc-scaffold example. Fixed.

@c12i c12i requested a review from matthme June 27, 2024 16:21
Copy link
Collaborator

@pdaoust pdaoust left a comment

Choose a reason for hiding this comment

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

I love all the new documentation! Just one question that may be important. Don't know whether that should be a 'comment' or 'request changes'.

Copy link
Collaborator

@matthme matthme left a comment

Choose a reason for hiding this comment

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

Great job! This PR is a nice improvement :)

@c12i c12i merged commit 465004f into holochain:develop Jun 28, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ShouldBackport/0.3 This change should be backported to develop-0.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Make scaffolding cli package manager agnostic
3 participants