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: Move Git identity check into ConfigManager and make more valid #3807

Merged
merged 1 commit into from Oct 20, 2019

Conversation

mih
Copy link
Member

@mih mih commented Oct 19, 2019

Also considers standard environment variables now.

Importantly, the check is no down without dedicated extra calls to git
config, saving two calls (that were pretty much always performed).

Fixes gh-3750

@mih mih added this to to-be-categorized in Release 0.12 via automation Oct 19, 2019
@mih mih moved this from to-be-categorized to Blocker in Release 0.12 Oct 19, 2019
"DataLad. Set both 'user.name' and 'user.email' "
"configuration variables."
)
ConfigManager._checked_git_identity = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/ConfigManager/self/ feels a bit more natural to me, but it's of course fine as is.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but I my personal preference is pointing the other way ;-)

@mih
Copy link
Member Author

mih commented Oct 20, 2019

Didnt realize we have a test run that sets identity variables. Will fix up

@codecov
Copy link

codecov bot commented Oct 20, 2019

Codecov Report

Merging #3807 into master will increase coverage by 10.3%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3807      +/-   ##
==========================================
+ Coverage   70.31%   80.61%   +10.3%     
==========================================
  Files         273      273              
  Lines       35954    35966      +12     
==========================================
+ Hits        25280    28993    +3713     
+ Misses      10674     6973    -3701
Impacted Files Coverage Δ
datalad/support/gitrepo.py 83.85% <ø> (+32.58%) ⬆️
datalad/__init__.py 78.94% <ø> (+55.24%) ⬆️
datalad/config.py 98.46% <100%> (+24.15%) ⬆️
datalad/tests/test_base.py 100% <100%> (+3.44%) ⬆️
datalad/core/local/tests/test_create.py 100% <0%> (ø) ⬆️
datalad/distribution/tests/test_publish.py 100% <0%> (ø) ⬆️
datalad/support/tests/test_gitrepo.py 99.88% <0%> (+0.1%) ⬆️
datalad/distribution/tests/test_install.py 99.79% <0%> (+0.2%) ⬆️
datalad/tests/test_utils.py 96.11% <0%> (+0.26%) ⬆️
datalad/interface/tests/test_rerun.py 100% <0%> (+0.4%) ⬆️
... and 84 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 4e6f671...5bd6aee. Read the comment docs.

Also considers standard environment variables now.

Importantly, the check is no down without dedicated extra calls to git
config, saving two calls (that were pretty much always performed).

Fixes dataladgh-3750
@mih
Copy link
Member Author

mih commented Oct 20, 2019

Thx @kyleam for the review. All good now.

@mih mih merged commit 1f37ebe into datalad:master Oct 20, 2019
Release 0.12 automation moved this from Blocker to done Oct 20, 2019
@mih mih deleted the bf-gh3750 branch October 20, 2019 08:33
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
Release 0.12
  
done
Development

Successfully merging this pull request may close these issues.

check_git_configured() performs inadequate check
2 participants