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: 295 monorepo directory structure design proposal #389

Open
wants to merge 62 commits into
base: main
Choose a base branch
from

Conversation

gvauter
Copy link
Member

@gvauter gvauter commented Nov 20, 2024

Description

This PR contains the initial code for a Click based CLI for trestle-bot as described in ADR-001. This Click based CLI will replace the existing entrypoints modules. This PR only contains the code for the CLI, it does not break any existing commands/actions. Subsequent PRs will make the necessary changes to swap the current entrypoints with these new CLI commands.

Fixes # #295

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this been tested?

Unit tests have been added in tests/cli for all new CLI commands/modules.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gvauter gvauter linked an issue Nov 20, 2024 that may be closed by this pull request
Copy link
Member Author

@gvauter gvauter left a comment

Choose a reason for hiding this comment

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

Some initial comments.

trestlebot/cli/base.py Outdated Show resolved Hide resolved
trestlebot/cli/base.py Outdated Show resolved Hide resolved
trestlebot/cli/commands/create.py Outdated Show resolved Hide resolved
trestlebot/cli/commands/create.py Outdated Show resolved Hide resolved
@qduanmu qduanmu force-pushed the 295-monorepo-directory-structure-design-proposal branch from 23ac47d to 0237e3b Compare November 21, 2024 07:39
@qduanmu qduanmu force-pushed the 295-monorepo-directory-structure-design-proposal branch from 0237e3b to 823e5bc Compare November 21, 2024 07:46
gvauter and others added 5 commits November 21, 2024 08:48
…thub.com:RedHatProductSecurity/trestle-bot into 295-monorepo-directory-structure-design-proposal
…thub.com:RedHatProductSecurity/trestle-bot into 295-monorepo-directory-structure-design-proposal
trestlebot/cli/commands/autosync.py Outdated Show resolved Hide resolved
trestlebot/cli/commands/autosync.py Outdated Show resolved Hide resolved
trestlebot/cli/commands/autosync.py Show resolved Hide resolved
trestlebot/cli/commands/autosync.py Outdated Show resolved Hide resolved
@click.option(
"--ssp-name",
prompt="Enter name of SSP to create",
help="Name of SSP to create.",
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, required=True should be set here. I'm not sure on the prompt. Today we do have a Github action for the create-cd command, so prompting would not work. It might be safest to let it error out and exit.

trestlebot/cli/commands/create.py Outdated Show resolved Hide resolved
tests/trestlebot/cli/test_create_cmd.py Outdated Show resolved Hide resolved
)
@click.option(
"--compdefs",
required=False,
Copy link
Member Author

Choose a reason for hiding this comment

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

For consistency I would vote we stick with the command sep list for now. I do believe is required for the create ssp command.

trestlebot/cli/commands/create.py Show resolved Hide resolved
trestlebot/cli/commands/init.py Outdated Show resolved Hide resolved
@hbraswelrh hbraswelrh self-requested a review December 4, 2024 03:45
@qduanmu qduanmu force-pushed the 295-monorepo-directory-structure-design-proposal branch from 8cd7085 to 1669ea8 Compare December 4, 2024 07:45
@gvauter gvauter requested review from jpower432 and qduanmu December 6, 2024 19:03
@gvauter gvauter marked this pull request as ready for review December 6, 2024 19:05
Copy link

@d10n d10n left a comment

Choose a reason for hiding this comment

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

Looks great

qduanmu
qduanmu previously approved these changes Dec 13, 2024
@qduanmu qduanmu dismissed their stale review December 13, 2024 05:40

I noted there are some missing options in create command.

@create_cmd.command(name="compdef", help="Component definition authoring subcommand.")
@click.pass_context
@common_create_options
@common_options
Copy link

Choose a reason for hiding this comment

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

@hbraswelrh , @git_options should be added here.

tmp_init_dir,
],
)
assert result.exit_code == 2

Choose a reason for hiding this comment

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

The expected exit_code is 2? Is there any context I missed?

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.

“Monorepo” Directory Structure Design Proposal
6 participants