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

Simplify git command, by letting exceptions go through #714

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Conversation

abravalheri
Copy link
Collaborator

@abravalheri abravalheri commented Apr 14, 2023

Purpose

This is an attempt to improve the UX when the git command is not installed.
As we can see in #712, the error might not be very intuitive to understand.

Approach

In this PR I am tried to simplify the code paths and let the shell exception happen.
(After looking at the commit history for the file I can see that we encountered in the past a lot of problems and with the we accumulated very sophisticated checks. It seems to me that we no longer need to return None in pyscaffold.shell.get_git_cmd, because now it will be executed lazily and therefore wait until the logging is properly configured).

@coveralls
Copy link

coveralls commented Apr 14, 2023

Coverage Status

Coverage: 97.919% (+0.07%) from 97.847% when pulling 2d0a40c on issue-712 into 83c7d5d on master.

@abravalheri
Copy link
Collaborator Author

I tested this manually using a windows container:

> docker run --rm -it python:3.10-windowsservercore-1809 powershell

cd $env:TEMP
Invoke-WebRequest -Uri https://github.com/pyscaffold/pyscaffold/archive/refs/heads/issue-712.zip -OutFile pyscaffold.zip
Expand-Archive -LiteralPath pyscaffold.zip -DestinationPath .
# Workaround for setuptools-scm
new-item pyscaffold-issue-712/.git_archival.txt
@"
node: 8c11fff53f49bd616bec72586e6e93ddb0c9c119
node-date: 2023-04-14T11:35:56+01:00
describe-name: v4.4.1b
ref-names: HEAD -> issue-712, origin/issue-712
"@ | add-content -Path pyscaffold-issue-712/.git_archival.txt
python -m pip install ./pyscaffold-issue-712
python -m pyscaffold.cli myproj
# Traceback (most recent call last):
#   File "C:\Python\lib\runpy.py", line 196, in _run_module_as_main
#     return _run_code(code, main_globals, None,
#   File "C:\Python\lib\runpy.py", line 86, in _run_code
#     exec(code, run_globals)
#   File "C:\Python\lib\site-packages\pyscaffold\cli.py", line 267, in <module>
#     main(sys.argv[1:])
#   File "C:\Python\lib\site-packages\pyscaffold\cli.py", line 256, in main
#     opts["command"](opts)
#   File "C:\Python\lib\site-packages\pyscaffold\cli.py", line 225, in run_scaffold
#     api.create_project(opts)
#   File "C:\Python\lib\site-packages\pyscaffold\api.py", line 156, in create_project
#     return reduce(actions.invoke, pipeline, ({}, opts))
#   File "C:\Python\lib\site-packages\pyscaffold\actions.py", line 99, in invoke
#     return action(*struct_and_opts)
#   File "C:\Python\lib\site-packages\pyscaffold\actions.py", line 222, in get_default_options
#     info.check_git()
#   File "C:\Python\lib\site-packages\pyscaffold\info.py", line 126, in check_git
#     raise GitNotInstalled
# pyscaffold.exceptions.GitNotInstalled: Make sure git is installed and working. Use flag --very-verbose for more information.

@abravalheri abravalheri marked this pull request as ready for review April 14, 2023 11:14
@AaronCanty
Copy link

Excellent! Will save future headaches :)

@abravalheri abravalheri merged commit 16cb9ab into master Jun 15, 2023
@abravalheri abravalheri deleted the issue-712 branch June 15, 2023 14:22
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