Skip to content
This repository has been archived by the owner. It is now read-only.

Make sbws actually read user-specified config if given #306

Closed
wants to merge 2 commits into from

Conversation

Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@pastly
Copy link
Member

@pastly pastly commented Dec 4, 2018

No description provided.

@@ -70,7 +70,8 @@ def _get_user_config(args, conf=None):
# sbws should start with a logger before reading configurations.
print('Configuration file %s not found, using defaults.' %
args.config)
return _extend_config(conf, args.config)
Copy link
Contributor

@juga0 juga0 Dec 7, 2018

I think it would be more clear if you return conf in the if

Suggested change
return _extend_config(conf, args.config)
return conf

@@ -70,7 +70,8 @@ def _get_user_config(args, conf=None):
# sbws should start with a logger before reading configurations.
print('Configuration file %s not found, using defaults.' %
args.config)
return _extend_config(conf, args.config)
else:
Copy link
Contributor

@juga0 juga0 Dec 7, 2018

i think it would be more clear if instead of else you return _extend_config here

Suggested change
else:
return _extend_config(conf, args.config)

Copy link
Member Author

@pastly pastly Dec 7, 2018

It would also cause us to try to read a file that doesn't exist.

Copy link
Contributor

@juga0 juga0 Dec 7, 2018

No when the return conf in #306 (comment) is applied first

@juga0
Copy link
Contributor

@juga0 juga0 commented Dec 7, 2018

Two commits to change to lines seems excesive to me

@juga0
Copy link
Contributor

@juga0 juga0 commented Dec 7, 2018

There's a test not finished, and would be great to add more tests for all the cases

@juga0
Copy link
Contributor

@juga0 juga0 commented Jan 9, 2019

Closing in favor of #326

@juga0 juga0 closed this Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.