tibirna / qgit

Official git repository for QGit.
Other
175 stars 68 forks source link

Properly handle GPG signatures in log, fix git version comparisons. #70

Open jaakristioja opened 5 years ago

jaakristioja commented 5 years ago

This fixes #66 and incorrect git version comparisons in src/git.cpp.

tibirna commented 5 years ago

Commit https://github.com/tibirna/qgit/pull/70/commits/ae562871fcb08 :

tibirna commented 5 years ago

Commit https://github.com/tibirna/qgit/pull/70/commits/d315fd45afd6 :

tibirna commented 5 years ago

Thank you very much for having taken the time to address #66.

I apologize for the long time I took for revising this. Please consider my comments and let me know. I will process your pull request immediately after.

jaakristioja commented 4 years ago

Sorry for the very long delay. To be honest, I stopped using qgit because I've learnt that the git command-line interface is actually sufficient for things I previously used qgit for (most importantly for visual graphs, e.g. git log --oneline --decorate --graph --no-show-signature --date-order --remotes --branches --tags). This means I'm no longer interested in pursuing this. I am sorry. I release my code in this pull request as public domain, no attribution required.

Note that commit https://github.com/tibirna/qgit/commit/ae562871fcb08 is not strictly required to fix #66, but is a different issue. Given that the git developers might not follow a strict versioning scheme, https://github.com/tibirna/qgit/commit/ae562871fcb08 might indeed not be enough.

As for gitVersionCompare() returning -1, 0 or 1, I tried to follow the logic of strcmp() and similar functions (including the <=> operator in C++20). If gitVersionCompare() is only to be used to compare against GIT_VERSION_REQUIRED, then perhaps it would make sense to refactor it into a bool isGitVersionCompatible(QString const &) function instead.

Regarding commit https://github.com/tibirna/qgit/commit/d315fd45afd6, I think that displaying a message for this would not provide any extra value, because you are already ignoring them if log.showSignature is not true in the git configuration. As opposed to --no-verify-signatures (as for git pull), passing --no-show-signature to git is just part of some background processing.

I hope this helps.

tibirna commented 4 years ago

Thank you very much for the detailed and thoroughly helpful rundown of your contribution. It is wholeheartedly appreciated.

With your accord, I will take at some point your branch, adapt it based on my observations and your clarifications, and included in the base qgit code.

Thanks again and great success in your future endeavors.

jaakristioja commented 4 years ago

Thank you very much! Same to you! :smiley: