Skip to content

Config should load as unmodified - #147

Merged
williammartin merged 2 commits into
trunkfrom
wm/entries-should-load-unmodified
Jan 23, 2024
Merged

williammartin merged 2 commits into
trunkfrom
wm/entries-should-load-unmodified

Conversation

@williammartin

Copy link
Copy Markdown
Member

Description

This PR relates to cli/cli#8496

Since the config was moved to go-gh this bug has been lurking around. The idea behind IsModified is that the config file shouldn't be written if only hosts has changed, and vice versa. Unfortunately, if there is a hosts file then on load of the files, the config entry would be SetModified as a result of having the hosts being added as an entry.

The consequence of this is that any call to Write would attempt to write the config file, even if only hosts had changed.

This PR sets the top level Config as unmodified to avoid that.

Testing notes

This command should not write to the config file:

gh config set --host github.com boo bar

You can set the config file as immutable on Mac like so:

sudo chflags uchg ~/.config/gh/config.yml

and revert it with:

sudo chflags nouchg ~/.config/gh/config.yml

uses: golangci/[email protected]
with:
version: v1.52
version: v1.54

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Bumped this because it seemed to resolve some linting errors. cli/cli is already on 1.54+

@andyfeller andyfeller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shipit-squirrel-thumbsup

@williammartin
williammartin merged commit d88d88f into trunk Jan 23, 2024
@williammartin
williammartin deleted the wm/entries-should-load-unmodified branch January 23, 2024 12:31
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