Conversation
Fix an error with SSL in some instances causing failures. Add methods to determine if there are local or remote changes only, in addition to checking for all changes. Add method to get the current revision of local git. Add method to read commit changes between local and remote. Update iGit documentation. Change fetch, run methods to return command output, versus throwing an error.
Merge piping to sdout.
|
Thanks for PR. I will review it during weekend, I don't have much time at the moment. |
janpecha
left a comment
janpecha
left a comment
There was a problem hiding this comment.
Hi, I added some comments, can you look at them? Thanks.
| { | ||
| $repository = dirname($repository); | ||
| // Fix ssl errors. | ||
| exec('git config --global http.sslVerify false'); |
There was a problem hiding this comment.
This is bad practise, SSL verify is usefull. Right way is fix SSL issue on server. Alternatively you can extends class GitRepository and change constructor in your code base:
class OwnGitRepository extends \Cz\Git\GitRepository
{
public function __construct($repository) {
parent::__construct($repository);
exec('git config --global http.sslVerify false');
}
}| $lastLine = exec('git status'); | ||
| $this->end(); | ||
| return (strpos($lastLine, 'nothing to commit')) === FALSE; // FALSE => changes | ||
| return ((strpos($lastLine, 'nothing to commit')) || (strpos($lastLine, 'branch is behind'))) === FALSE; |
There was a problem hiding this comment.
This is BC break. Method hasChanges() means "has changes to commit" (I will update phpdoc).
What is your use-case? In what case is last line "branch is behind"? Can you give a example? Probably better will be new method, something like your suggested hasRemoteChanges() or isRemoteSynced().
| $this->end(); | ||
| return $commits; | ||
| } | ||
| public function getRev() { |
There was a problem hiding this comment.
Probably better is getCurrentRevision()? It's consistent with getCurrentBranch().
| $this->begin(); | ||
| $result = $this->run("git pull $remote", $params); | ||
| $this->end(); | ||
| return $result; |
There was a problem hiding this comment.
It's BC break. Why you need git pull output?
| $this->begin(); | ||
| $result = $this->run("git fetch $remote", $params); | ||
| $this->end(); | ||
| return $result; |
There was a problem hiding this comment.
It's BC break. Why you need git fetch output?
| $shortHead = exec($command.'"%h"'); | ||
| $subject = exec($command.'"%s"'); | ||
| exec($command.'"%b"',$body); | ||
| $body = implode('<br>',$body); |
| $author = exec($command.'"%aN"'); | ||
| $date = exec($command.'"%aD"'); | ||
| $commit = [ | ||
| 'head'=>$head, |
There was a problem hiding this comment.
Maybe hash is better than head.
| $date = exec($command.'"%aD"'); | ||
| $commit = [ | ||
| 'head'=>$head, | ||
| 'shortHead'=>$shortHead, |
There was a problem hiding this comment.
Is shortHead really needed? It can be obtained from hash.
| $this->begin(); | ||
| $commits = []; | ||
| if (!is_numeric($limit)) { | ||
| $command = "git log $limit..$branch --oneline"; |
There was a problem hiding this comment.
Values of $limit and $branch must be escaped - use self::processCommand() and maybe add ' 2>&1' (it redirects error output to STDOUT).
| 'subject'=>$subject, | ||
| 'body'=>$body, | ||
| 'author'=>$author, | ||
| 'date'=>$date |
There was a problem hiding this comment.
I think better is date in format like 2013-08-18T15:34:40+00:00 (--date=iso-strict) or 2013-09-12 09:43:52 +0000 (--date=iso) than default Sun Aug 18 14:49:42 2013 +0000.
| $commits = []; | ||
| if (!is_numeric($limit)) { | ||
| $command = "git log $limit..$branch --oneline"; | ||
| exec($command,$shorts); |
There was a problem hiding this comment.
What if exec fails (for example if git is not installed)?
2 participants
No description provided.