2: Refactoring all internal booleans :star:

rycus86 / githooks

Githooks: per-repo and global Git hooks with version control

MIT License

382 stars 20 forks source link

2: Refactoring all internal booleans :star: #100

Closed gabyx closed 4 years ago

gabyx commented 4 years ago

No arguments over environment anymore Backward compatible for update!
Change all "yes" -> "true" to make it more git config like git config --bool and consistent.

4 Commits changing variables...

gabyx commented 4 years ago

Thx for merging.

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 648

57 of 60 (95.0%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.03%) to 83.768%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
install.sh	36	39	92.31%
<!--	Total:	57	60	95.0%	-->

Totals
Change from base Build 643:	-0.03%
Covered Lines:	2090
Relevant Lines:	2495

💛 - Coveralls

gabyx commented 4 years ago

Can you have a look at this. This would be good to go.

rycus86 commented 4 years ago

How are the Y -> true and N -> false changes backwards compatible? I think they would just be invalid value and probably be handled on the fallback branches of the conditions. Could you perhaps revert all the test changes and see that everything still works? That way we'd have better confidence that these changes are actually backwards compatible. Thanks!

gabyx commented 4 years ago

Ah jeah that might be a good idea,

gabyx commented 4 years ago

The changes are backward compatible (IMO) because all tests are adapted such that they still test the old values

[ "$A" = "true"] || [ "$A" = "yes" ] || return 1

gabyx commented 4 years ago

Pushed the necessary tests changes to check for backward compatibility!

gabyx commented 4 years ago

If all tests pass I will add the more changes to all tests to update them.

gabyx commented 4 years ago

All tests in : https://travis-ci.org/github/rycus86/githooks/builds/695011929 work. Meaning backward compatibility should be :crossed_fingers: ok.

I will add the other tests changes back in.

gabyx commented 4 years ago

Lets see if all tests pass now, if so, Its good to go :+1:

gabyx commented 4 years ago

A next PR would be:

cli.sh from release clone or
remove the nasty version number update in the files (always gives merge conflicts). should work with installs from very old versions... (we leave the version number in, for those who read this number), its complicated -> maybe later ...

gabyx commented 4 years ago

Ok, everythin works except arch of course -> Good to go :+1:

rycus86 commented 4 years ago

Thanks for this! 👌 For what's next:

cli.sh from release clone

Could we please do this first to see how that goes?

remove the nasty version number update in the files (always gives merge conflicts). should work with installs from very old versions... (we leave the version number in, for those who read this number), its complicated -> maybe later ...

I kind of like that version number to be checked in and available in the local file. I agree the merge conflict is somewhat annoying, perhaps we could work something out to keep the version number but make it less annoying? Any ideas?