Skip to content

feat(move): Project-PHID as a CLI argument for the move command#9

Merged
brouberol merged 5 commits into
brouberol:mainfrom
gehel:project_phid_as_argument
Oct 3, 2025
Merged

feat(move): Project-PHID as a CLI argument for the move command#9
brouberol merged 5 commits into
brouberol:mainfrom
gehel:project_phid_as_argument

Conversation

@gehel

@gehel gehel commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

When working with multiple projects, it is cumbersome to edit config file or environment variables each time a different project is needed. This commit introduces a "--project-phid" argument to the "move" command, which would override
"phabricator_default_project_phid".

It might make sense in a next step to introduce the same "--project-phid" argument to other commands where it makes sense.

@brouberol brouberol left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good! I have a couple of simplification suggestions.

Comment thread phable/cli/move.py Outdated
Comment thread phable/cli/move.py Outdated
Comment thread phable/cli/utils.py
Comment thread phable/cli/move.py Outdated
@gehel gehel force-pushed the project_phid_as_argument branch 2 times, most recently from 77aaa92 to a28a05c Compare September 11, 2025 12:42
@brouberol

Copy link
Copy Markdown
Owner

Actually, 💡 . People don't deal in PHIDs. They deal in project names. Let's maybe replace --project-phid by --project or --project-name and perform a project search by name, via https://github.com/brouberol/phable/blob/main/phable/phabricator.py#L334-L345 ?

When working with multiple projects, it is cumbersome to edit
config file or environment variables each time a different
project is needed. This commit introduces a "--project-phid" argument
to the "move" command, which would override
"phabricator_default_project_phid".

It might make sense in a next step to introduce the same
"--project-phid" argument to other commands where it makes sense.
Now that we have extracted a standard way to passing `project_phid`,
let's use it on the "list" command as well. See parent commit for
details.
Now that we have extracted a standard way to passing `project_phid`,
let's use it on the "parent" command as well. See parent commit for
details.
@gehel gehel force-pushed the project_phid_as_argument branch from 9af5788 to 3c90f00 Compare September 12, 2025 08:41
@gehel

gehel commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

Actually, 💡 . People don't deal in PHIDs. They deal in project names. Let's maybe replace --project-phid by --project or --project-name and perform a project search by name, via https://github.com/brouberol/phable/blob/main/phable/phabricator.py#L334-L345 ?

I lazy and did not want to implement project search. But if it exists already, we should use it! PHIDs are not for humans!

If we use project names instead of PHID, should we also use those in the config file?

In any case, could we do this work in a subsequent PR?

@gehel

gehel commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

I lazy and did not want to implement project search. But if it exists already, we should use it! PHIDs are not for humans!

Arguments are now project and not PHIDs

If we use project names instead of PHID, should we also use those in the config file?

Let's keep this one for a different CR.

Comment thread phable/cli/move.py Outdated
Comment thread phable/cli/report.py Outdated
Humans should not be dealing with PHIDs. Let's pass project title
instead and resolve them to PHID by doing a search.
@gehel gehel force-pushed the project_phid_as_argument branch from 242881d to 6fecb68 Compare September 12, 2025 12:30
@gehel

gehel commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

Implemented the last suggestion by also extracting a find_project_phid_by_title() function to reduce duplication. This is in cli/utils.py at the moment, but it screams that a class is needed to store the configuration (PhabricatorClient and click.Context). Maybe I'll introduce this in another commit.

@gehel

gehel commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

Attempt to introduce that class: gehel@b941467. But it does not work, I don't quite understand how python does method handles...

Comment thread phable/cli/move.py Outdated
Co-authored-by: Balthazar Rouberol <br@imap.cc>
@brouberol

Copy link
Copy Markdown
Owner

Thanks!

@brouberol brouberol merged commit 40da342 into brouberol:main Oct 3, 2025
@gehel gehel deleted the project_phid_as_argument branch October 3, 2025 12:35
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