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

Merged
merged 137 commits into from
Dec 18, 2024

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
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.

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?

Copy link

@huiwangredhat huiwangredhat Dec 17, 2024

Choose a reason for hiding this comment

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

I think the arguments, '--committer-email', '--committer-name' and '--branch'could be added; repo-path could be updated to --repo-path; theassert result.exit_code == 2could beresult.exit_code == 0 `.

Copy link
Member Author

Choose a reason for hiding this comment

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

@huiwangredhat yes, you are correct, this should have a successful exit code since test expects it work correctly.

Copy link

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

Only one typo and other two minor empty lines. I didn't check all transformations but only the CLI changes.

If the user specifies a value for the option directly (e.g. uses --option value)
then that value is used in favor of the value loaded from the config.

Simarly, if an option has an associated ENVVAR, and that ENVVAR is set, then the

Choose a reason for hiding this comment

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

Suggested change
Simarly, if an option has an associated ENVVAR, and that ENVVAR is set, then the
Similarly, if an option has an associated ENVVAR, and that ENVVAR is set, then the

@@ -0,0 +1,59 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright (c) 2023 Red Hat, Inc.

Choose a reason for hiding this comment

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

Suggested change

@@ -0,0 +1,36 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright (c) 2024 Red Hat, Inc.

Choose a reason for hiding this comment

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

Suggested change

Comment on lines 51 to 52
help="Comma-separated list of upstream git sources to sync. Each source is a string \
in the form <repo_url>@<ref> where ref is a git ref such as a tag or branch.",

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 list of upstream git sources to sync. Each source is a string \
in the form <repo_url>@<ref> where ref is a git ref such as a tag or branch.",
help="Comma-separated list of upstream git sources to sync. Each source is a string \
in the form <repo_url>@<ref> where ref is a git ref such as a tag or branch.",

Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Signed-off-by: George Vauter <gvauter@redhat.com>
Copy link

@huiwangredhat huiwangredhat left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for updating.

@qduanmu qduanmu force-pushed the 295-monorepo-directory-structure-design-proposal branch from 18eaf09 to 769d0df Compare December 18, 2024 03:55
Copy link

@marcusburghardt marcusburghardt 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. I think this new CLI improved considerably the experience. Thanks.

…thub.com:RedHatProductSecurity/trestle-bot into 295-monorepo-directory-structure-design-proposal
@gvauter gvauter merged commit 0314389 into main Dec 18, 2024
13 checks passed
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
7 participants