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

BF: config - become consistent with git in treating key without any value as True #4421

Merged
merged 1 commit into from Apr 18, 2020

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Apr 17, 2020

In our case these changes will

  • result in None being the value
  • getbool returning True (really adhoc, not part of any2bool since
    makes little sense generally)

Without this fix our code would simply crash while reading a legit (to
git) .git/config with

  File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 301, in reload
    log_stderr=True
  File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 96, in _parse_gitconfig_dump
    kv_match = cfg_kv_regex.match(line)
AttributeError: 'NoneType' object has no attribute 'groups'

A little more on this messy issue of boolean values could be found
in a recent discussion on git-annex:

https://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/

…alue as True

In our case these changes will

- result in `None` being the value
- `getbool` returning `True` (really adhoc, not part of any2bool since
  makes little sense generally)

Without this fix our code would simply crash while reading a legit (to
git) .git/config with

      File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 301, in reload
        log_stderr=True
      File "/home/yoh/proj/datalad/datalad-maint/datalad/config.py", line 96, in _parse_gitconfig_dump
        kv_match = cfg_kv_regex.match(line)
    AttributeError: 'NoneType' object has no attribute 'groups'

A little more on this messy issue of boolean values could be found
in a recent discussion on git-annex:

https://git-annex.branchable.com/bugs/does_not_understand___34__yes__34___as_boolean_true_value_for_autoenable/
kyleam
kyleam approved these changes Apr 17, 2020
@codecov
Copy link

@codecov codecov bot commented Apr 17, 2020

Codecov Report

Merging #4421 into maint will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #4421      +/-   ##
==========================================
- Coverage   89.70%   89.68%   -0.02%     
==========================================
  Files         275      275              
  Lines       37055    37505     +450     
==========================================
+ Hits        33240    33638     +398     
- Misses       3815     3867      +52     
Impacted Files Coverage Δ
datalad/config.py 96.59% <100.00%> (+0.05%) ⬆️
datalad/tests/test_config.py 99.17% <100.00%> (+0.01%) ⬆️
datalad/downloaders/http.py 72.11% <0.00%> (-2.79%) ⬇️
datalad/downloaders/tests/test_http.py 58.79% <0.00%> (-2.17%) ⬇️
datalad/interface/tests/test_unlock.py 96.19% <0.00%> (-1.91%) ⬇️
datalad/core/local/tests/test_save.py 96.26% <0.00%> (-0.50%) ⬇️
datalad/support/tests/test_annexrepo.py 95.27% <0.00%> (-0.43%) ⬇️
datalad/tests/utils.py 89.50% <0.00%> (-0.35%) ⬇️
datalad/support/annexrepo.py 86.03% <0.00%> (-0.25%) ⬇️
datalad/utils.py 83.97% <0.00%> (-0.10%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa2da5c...b733b04. Read the comment docs.

mih
mih approved these changes Apr 18, 2020
Copy link
Member

@mih mih left a comment

Cool, thx!

@mih mih merged commit f6d9561 into datalad:maint Apr 18, 2020
10 of 11 checks passed
@yarikoptic yarikoptic added this to the 0.12.6 milestone Apr 23, 2020
@yarikoptic yarikoptic deleted the bf-bool-novalue branch May 21, 2020
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.

None yet

3 participants