Skip to content

Fix getting config from git config#79

Open
fallen wants to merge 1 commit intogetpatchwork:mainfrom
fallen:main
Open

Fix getting config from git config#79
fallen wants to merge 1 commit intogetpatchwork:mainfrom
fallen:main

Conversation

@fallen
Copy link
Copy Markdown

@fallen fallen commented Apr 15, 2026

Currently the CLI will store to the Config object
values from the argument parsing regardless of
whether they are None or not.

But if config.xxx is created, even with a None value the __getattr__ will never get called and thus will never lookup the git config value.

The fix is to only store non-None values from the
argument parsing to the Config object.

Without this patch, doing git pw series list will never work, even if server and project are configured in git config

Currently the CLI will store to the Config object
values from the argument parsing regardless of
whether they are None or not.

But if config.xxx is created, even with a None value
the __getattr__ will never get called and thus will
never lookup the git config value.

The fix is to only store non-None values from the
argument parsing to the Config object.
Copy link
Copy Markdown
Member

@stephenfin stephenfin left a comment

Choose a reason for hiding this comment

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

Thanks, this is a good catch. I have some small requests inlone

Comment thread git_pw/shell.py
Comment on lines +100 to +111
if debug:
CONF.debug = debug
if token:
CONF.token = token
if username:
CONF.username = username
if password:
CONF.password = password
if server:
CONF.server = server
if project:
CONF.project = project
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you make these explicit None checks rather than false'y? You will also need to change the default for --debug to None and update the hint to bool | None

Suggested change
if debug:
CONF.debug = debug
if token:
CONF.token = token
if username:
CONF.username = username
if password:
CONF.password = password
if server:
CONF.server = server
if project:
CONF.project = project
if debug is not None:
CONF.debug = debug
if token is not None:
CONF.token = token
if username is not None:
CONF.username = username
if password is not None:
CONF.password = password
if server is not None:
CONF.server = server
if project is not None:
CONF.project = project

I also wonder if it's possible to add a small test to prevent this regressing?

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