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

Renaming doWithCommandeer to cfgInit to make clearer what it does and for consistency #6881

Closed
wants to merge 3 commits into from
Closed

Conversation

MarkRosemaker
Copy link
Contributor

@MarkRosemaker MarkRosemaker commented Feb 11, 2020

Regarding my proposal to simplify doWithCommandeer, I found that renaming alone will clear things up.

For one, I realized that there are two different object types:

  1. doWithCommandeer func(c *commandeer) error
  2. doWithCommandeer func(cfg config.Provider) error

When initializeConfig(mustHaveConfigFile, running bool, h *hugoBuilderCommon, f flagsToConfigHandler, doWithCommandeer func(c *commandeer) error) is called, the last argument is either nil or was named cfgInit.

Therefore, renaming 1. to cfgInit clears things up because it's the only thing it ever does.
I renamed 2. cfgSetAndInit because it sets the Cfg field of commandeer (to a *viper.Viper pointer) and runs cfgInit.

@claassistantio
Copy link

claassistantio commented Feb 11, 2020

CLA assistant check
All committers have signed the CLA.

@bep
Copy link
Member

bep commented Feb 11, 2020

You need to gofmt your source files. I recommend you use an editor where you can configure this to happen automatically on save. I use LiteIDE where this is default.

@MarkRosemaker
Copy link
Contributor Author

That's strange. I do have gofmt on save and I now manually ran it. There are no changes to commit.

@bep
Copy link
Member

bep commented Feb 11, 2020

What Go version are you running? We do the "gofmt check" on Travis with Go 1.13 only as they tend to change their format rules slightly from version to version. So if you gofmt with Go 1.12 it could still be "wrong" for Go 1.13.

@MarkRosemaker
Copy link
Contributor Author

What Go version are you running? We do the "gofmt check" on Travis with Go 1.13 only as they tend to change their format rules slightly from version to version. So if you gofmt with Go 1.12 it could still be "wrong" for Go 1.13.

Fixed it, thanks!

@bep
Copy link
Member

bep commented Feb 12, 2020

OK, the change itself looks good, but can you squash the 3 commits and make a proper commit message (it does not have to be long, important part is the title).

@jtierney4

This comment has been minimized.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants