diff mbox series

[3/5] git: Remove is None check on _find_git_lfs

Message ID 20230217170009.58707-3-paulo@myneves.com
State New
Headers show
Series [1/5] tests: git-lfs: Restore _find_git_lfs | expand

Commit Message

Paulo Neves Feb. 17, 2023, 5 p.m. UTC 
shutil.which returns None when the argument is not found.
As per documentation[1] None is evaluated to False and everything
else is True, so it is safe to have _find_git_lfs just return
the value of shutil.which

[1] https://docs.python.org/3/library/stdtypes.html#truth-value-testing

Signed-off-by: Paulo Neves <paulo@myneves.com>
---
 lib/bb/fetch2/git.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.34.1

Comments

Richard Purdie Feb. 22, 2023, 12:03 p.m. UTC | #1 
On Fri, 2023-02-17 at 17:00 +0000, Paulo Neves wrote:
> shutil.which returns None when the argument is not found.
> As per documentation[1] None is evaluated to False and everything
> else is True, so it is safe to have _find_git_lfs just return
> the value of shutil.which
> 
> [1] https://docs.python.org/3/library/stdtypes.html#truth-value-testing
> 
> Signed-off-by: Paulo Neves <paulo@myneves.com>
> ---
>  lib/bb/fetch2/git.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index 2e3d32515..a3f29d2b9 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -687,7 +687,7 @@ class Git(FetchMethod):
>          Return True if git-lfs can be found, False otherwise.
>          """
>          import shutil
> -        return shutil.which("git-lfs", path=d.getVar('PATH')) is not None
> +        return shutil.which("git-lfs", path=d.getVar('PATH'))
> 
>      def _get_repo_url(self, ud):
>          """

I don't agree with this change since currently the function does return
True/False as it says in the docstring but after your change, it
returns other things.

Yes, evaluating these in python will result in True/False and that is
safe but that isn't what it is documented to return.

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 2e3d32515..a3f29d2b9 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -687,7 +687,7 @@  class Git(FetchMethod):
         Return True if git-lfs can be found, False otherwise.
         """
         import shutil
-        return shutil.which("git-lfs", path=d.getVar('PATH')) is not None
+        return shutil.which("git-lfs", path=d.getVar('PATH'))

     def _get_repo_url(self, ud):
         """