Skip to content

crt: add option to run local#21

Merged
jecluis merged 4 commits intoclyso:mainfrom
UweSchwaeke:wip-crt-local-run
Mar 12, 2026
Merged

crt: add option to run local#21
jecluis merged 4 commits intoclyso:mainfrom
UweSchwaeke:wip-crt-local-run

Conversation

@UweSchwaeke
Copy link
Collaborator

@UweSchwaeke UweSchwaeke commented Jan 22, 2026

crt: introduce --local-run option

  • what:
    introduce an option flag signaling the crt tool to run locally. "locally" means the tool will not access remote repositories. commands and subcommands that explicitly require remote access will simply ignore the flag.

  • why:
    this enables the crt tool to operate without a connection to remote repositories or the internet. it allows users to perform preparatory tasks—such as starting a release adding, removing manifests or adding patches, using only local data. a network connection is now only strictly required when finalizing the release.

@UweSchwaeke UweSchwaeke requested a review from jecluis January 22, 2026 12:55
Copy link
Contributor

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

@UweSchwaeke I've left some comments on the various commits.

Additionally, we need a proper description on this Pull Request, including what you are attempting to do. I'm a bit lost on what you are actually trying to achieve (maybe I forgot some of our initial chat on this).

So it would be nice to have the context on the PR description so we know what we are aiming for.

@UweSchwaeke
Copy link
Collaborator Author

@UweSchwaeke I've left some comments on the various commits.

Additionally, we need a proper description on this Pull Request, including what you are attempting to do. I'm a bit lost on what you are actually trying to achieve (maybe I forgot some of our initial chat on this).

So it would be nice to have the context on the PR description so we know what we are aiming for.

I added a description.

@jecluis
Copy link
Contributor

jecluis commented Jan 29, 2026

@UweSchwaeke on the commits' subjects, I'll need you to add a colon (:) between the component being changed and the subject.

@UweSchwaeke
Copy link
Collaborator Author

@UweSchwaeke on the commits' subjects, I'll need you to add a colon (:) between the component being changed and the subject.

just a moment i will fix it

@jecluis
Copy link
Contributor

jecluis commented Jan 29, 2026

Also, please check the commit messages for typos. One example, the first commit shows:

don't check if release allready exists in remote

s/allready/already/

Additionally, ensure the commit messages are properly capitalized where it makes sense. While we don't want subject lines to be capitalized (unless required, e.g., to describe a CONSTANT_NAME), the commit message itself should come as easy-to-read text. I would also suggest adding a blank line between the commit's subject and the commit message body.

@UweSchwaeke
Copy link
Collaborator Author

Also, please check the commit messages for typos. One example, the first commit shows:

don't check if release allready exists in remote

s/allready/already/

Additionally, ensure the commit messages are properly capitalized where it makes sense. While we don't want subject lines to be capitalized (unless required, e.g., to describe a CONSTANT_NAME), the commit message itself should come as easy-to-read text. I would also suggest adding a blank line between the commit's subject and the commit message body.

i added a better description ( mostly reviewed with gemini)

@UweSchwaeke UweSchwaeke marked this pull request as ready for review January 30, 2026 05:43
@UweSchwaeke UweSchwaeke requested a review from jecluis February 3, 2026 19:36
Copy link
Contributor

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

This looks sound, but I think we'll want to add some sort of functional tests to go along with it to make sure and validate.

is_flag=True,
default=False,
required=False,
help="run without accessing remotes",
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize "Run", add a period (.) at the end, following what other options' help messages are doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done thanks

summary_table.add_row("From Base Reference", f"{base_ref} from {base_ref_repo}")
summary_table.add_row("Release base branch", release_base_branch)
summary_table.add_row("Release base tag", release_base_tag)
summary_table.add_row("Is local", str(run_locally))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure if we should make this conditional. I wonder if it will be confusing for someone who's not using it as a local tool.

Copy link
Collaborator Author

@UweSchwaeke UweSchwaeke Feb 12, 2026

Choose a reason for hiding this comment

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

You mean, if someone let the script run through ssh or if its get called by a server or something?
Maybe i could make 2 Options:

  1. Remove the line, so its quiet and the user doesn't get informed.
  2. Rename "Is local" to something like "Ran without access to Remote repositories" and make it not part of the table instead a pinfo output.

What do you prefer? Or am i missing another option?

@UweSchwaeke UweSchwaeke force-pushed the wip-crt-local-run branch 6 times, most recently from 7a200a6 to 5b1511b Compare March 11, 2026 11:29
avoid accessing remotes if the flag is set and the subcommand doesn't require remote access.
affected subcommands:

* start:
  * skip adding remote URLs.
  * skip checking if the release already exists on the remote.
  * create a local release branch from base_ref (which must exist locally).
  * do not push the created branch or tag to the remote.

* list:
  * skip adding remote urls.
  * skip fetching from remotes.

other subcommands will ignore the flag.

Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
@UweSchwaeke UweSchwaeke force-pushed the wip-crt-local-run branch 4 times, most recently from ef460ae to 44ebc4d Compare March 11, 2026 12:50
don't access remotes if the flag is set and the subcommand doesn't require remote access.
affected subcommands:

* patchset add:
  * don't add remote urls and don't fetch patch branch (assume branch exists locally)

* validate:
  * don't add remote urls and don't fetch from remote

other subcommands will ignore the flag

Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
@UweSchwaeke UweSchwaeke force-pushed the wip-crt-local-run branch 2 times, most recently from 1e2f124 to f95accd Compare March 11, 2026 13:10
don't access remotes if the flag is set and the subcommand doesn't require remote access.
affected subcommands:
* add:
  * don't add remote url and don't fetch

Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
don't access remotes if the flag is set and the subcommand doesn't require remote access.
affected subcommands:

* add:
   * don't add remote url and don't fetch
   * create branch from an existing local branch

 * publish:
   * don't add remote url and don't fetch
   * create branch from an existing local branch

other subcommands will ignore the flag

Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
@jecluis jecluis merged commit a3d13d7 into clyso:main Mar 12, 2026
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.

2 participants