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

fix a no-used var #22

Merged
merged 1 commit into from
Feb 19, 2020
Merged

fix a no-used var #22

merged 1 commit into from
Feb 19, 2020

Conversation

hjlarry
Copy link
Contributor

@hjlarry hjlarry commented Feb 18, 2020

seems declared but not use it

@thblt
Copy link
Owner

thblt commented Feb 18, 2020

Thank you! Could you please also update the source file, write-yourself-a-git.org?

@hjlarry
Copy link
Contributor Author

hjlarry commented Feb 18, 2020

ok, by the way,

hashRE = re.compile(r"^[0-9A-Fa-f]{1,16}$")
...
if hashRE.match(name):
        if len(name) == 40:
            # This is a complete hash
            return [name.lower()]

This is not work for me. Is it should be re.compile(r"^[0-9A-Fa-f]{1,40}$") ?

@thblt
Copy link
Owner

thblt commented Feb 18, 2020

Err, of course, thanks again! Actually, it should even be

re.compile(r"^[0-9A-Fa-f]{4,40}$")

@thblt
Copy link
Owner

thblt commented Feb 18, 2020

(If you can, could you add a test to check that 40-char hashes work?)

@hjlarry
Copy link
Contributor Author

hjlarry commented Feb 18, 2020

ok, I will do that later

@hjlarry
Copy link
Contributor Author

hjlarry commented Feb 19, 2020

done!

@thblt thblt merged commit 674c8cc into thblt:master Feb 19, 2020
@thblt
Copy link
Owner

thblt commented Feb 19, 2020

Thanks again!

@hjlarry hjlarry deleted the patch-1 branch February 19, 2020 09:51
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.

2 participants