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

feat(cmds): allow to set the configuration file path #8634

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

schomatis
Copy link
Contributor

@schomatis schomatis commented Jan 3, 2022

First iteration, still incomplete. Review needed at this point to move forward.

Depends on ipfs/go-ipfs-config#162.

Closes #8067.

@schomatis schomatis self-assigned this Jan 3, 2022
core/commands/root.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/ipfsfetcher/ipfsfetcher.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/migrations.go Outdated Show resolved Hide resolved
@schomatis schomatis force-pushed the schomatis/feat/cmds/use-custom-config-file-path branch from f64d1b0 to 0764066 Compare February 21, 2022 22:39
@schomatis schomatis marked this pull request as ready for review February 21, 2022 23:12
@BigLep BigLep added this to the Best Effort Track milestone Mar 3, 2022
@BigLep
Copy link
Contributor

BigLep commented Mar 5, 2022

@guseggert : can you do the final review/merge so we can close this out?

core/commands/root.go Outdated Show resolved Hide resolved
core/commands/root.go Outdated Show resolved Hide resolved
go.mod Outdated
Comment on lines 34 to 35
// FIXME(BLOCKING): Update to release tag once https://github.com/ipfs/go-ipfs-config/pull/162 lands.
github.com/ipfs/go-ipfs-config v0.19.1-0.20220221223623-ea7b3a0b7fba
Copy link
Contributor

Choose a reason for hiding this comment

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

update

return &FSRepo{path: expPath}, nil
configFilePath, err := config.Filename(rpath, userConfigFilePath)
if err != nil {
// FIXME: Personalize this when the user config path is "".
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not blocking, just a normal FIXME, can remove if you want.

repo/fsrepo/fsrepo.go Outdated Show resolved Hide resolved
repo/fsrepo/fsrepo.go Outdated Show resolved Hide resolved
repo/fsrepo/fsrepo.go Outdated Show resolved Hide resolved
repo/fsrepo/fsrepo.go Outdated Show resolved Hide resolved
@lidel

This comment was marked as resolved.

@schomatis schomatis force-pushed the schomatis/feat/cmds/use-custom-config-file-path branch 3 times, most recently from 5fa000a to 34356a1 Compare March 9, 2022 17:57
@schomatis schomatis requested a review from guseggert March 9, 2022 17:57
@guseggert
Copy link
Contributor

Can you add a sharness test covering these new flags?

@BigLep
Copy link
Contributor

BigLep commented Mar 14, 2022

@guseggert : it says you approved the changes but I see you've also requested a sharness test. To make it clear, do you want to adjust your review to "request changes"?

Copy link
Contributor

@guseggert guseggert left a comment

Choose a reason for hiding this comment

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

Can you add a sharness test covering these new flags?

@schomatis schomatis force-pushed the schomatis/feat/cmds/use-custom-config-file-path branch from 34356a1 to 3f4c0df Compare March 25, 2022 00:09
@schomatis
Copy link
Contributor Author

Not sure what are the ci/circleci: go-ipfs-http-client fails about.

@guseggert
Copy link
Contributor

Not sure what are the ci/circleci: go-ipfs-http-client fails about.

It's a known issue, we can ignore

@BigLep
Copy link
Contributor

BigLep commented Apr 22, 2022

@schomatis : can you rebase please? That will fix CI and then we can merge.

@schomatis schomatis force-pushed the schomatis/feat/cmds/use-custom-config-file-path branch from ec9bcec to 9a8cb13 Compare April 22, 2022 15:54
@schomatis schomatis force-pushed the schomatis/feat/cmds/use-custom-config-file-path branch from 9a8cb13 to e172ad2 Compare April 22, 2022 16:03
@schomatis schomatis merged commit d6077b8 into master Apr 22, 2022
@schomatis schomatis deleted the schomatis/feat/cmds/use-custom-config-file-path branch April 22, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

--config expects directory not file
4 participants