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: Init command #780

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

feat: Init command #780

wants to merge 5 commits into from

Conversation

exaby73
Copy link

@exaby73 exaby73 commented Oct 23, 2024

Closes #773

Description

Adds init command to melos

Type of Change

  • feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
  • ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 refactor -- Code refactor
  • ci -- Build configuration change
  • 📝 docs -- Documentation
  • 🗑️ chore -- Chore

Copy link

docs-page bot commented Oct 23, 2024

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/melos~780

Documentation is deployed and generated using docs.page.

Copy link
Collaborator

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Looking forward to this feature!

packages/melos/lib/src/command_runner/init.dart Outdated Show resolved Hide resolved
argParser.addMultiOption(
'packages',
abbr: 'p',
help: 'Comma separated packages to add in top level `packages` array',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
help: 'Comma separated packages to add in top level `packages` array',
help: 'Comma separated packages to add in top level `packages` array.',

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one should probably be a bit more descriptive, "add in top level packages array" doesn't really say much to the user.

Copy link
Author

Choose a reason for hiding this comment

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

Take a look at what I have now "Comma separated glob paths to add to the melos workspace."

packages/melos/lib/src/command_runner/init.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/command_runner/init.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/init.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/init.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/init.dart Show resolved Hide resolved
packages/melos/lib/src/commands/init.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/runner.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/runner.dart Show resolved Hide resolved
Comment on lines +51 to +55
final project = argResults!['project'] as String? ??
promptInput(
'Enter the project name',
defaultsTo: workspaceName,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that we need to differentiate between workspace name and project name


logger.log(
'Initialized Melos workspace in ${dir.path}.\n'
'Run the following commands to bootstrap the workspace:\n'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'Run the following commands to bootstrap the workspace:\n'
'Run the following commands to bootstrap the workspace when you have created some packages and/or apps:\n'

Since this won't do anything when run directly after init has been run we should explain that they should run this once the apps and/or packages are created.

logger.log(
'Initialized Melos workspace in ${dir.path}.\n'
'Run the following commands to bootstrap the workspace:\n'
' cd ${dir.path}\n'
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the path is . we should omit this

packages/melos/lib/src/commands/runner.dart Show resolved Hide resolved
@@ -84,6 +87,7 @@ class Melos extends _Melos

abstract class _Melos {
MelosLogger get logger;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

defaultsTo: workspaceName,
);
final useAppsDir = promptBool(
message: 'Do you want to add the apps directory to the list of packages?',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
message: 'Do you want to add the apps directory to the list of packages?',
message: 'Do you want to add the apps directory?',

We are actually creating the apps directory right? I don't think we need to specify that it will be added in the melos.yaml file too, and the wording as is is slightly confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request: Add init command to bootstrap a melos project
2 participants