twmr / gerrit.el

gerrit integration in emacs
44 stars 5 forks source link

Replace usage of `git-review` cli tool by gerrit REST api #10

Closed twmr closed 3 years ago

twmr commented 4 years ago

Since it doesn't seem that #2 will soon be fixed in gerrit and in git-review and since not everyone is using git-review, we should remove the usage of git-review from gerrit.el and use REST calls instead.

The user-interface of gerrit-download and gerrit-upload should be kept and only slightly adapted.

DONE which features of git-review do we want to implement in gerrit.el? DONE how do we determine the target branch for a to-be-uploaded gerrit-change? git-review reads .gitreview file, which may contain a branch name that is used for gerrit changes. DONE who sets-up the pre-commit hooks that ensure that a "Change-ID" line is part of the commit message?

gerrit-download

This implementation could be used instead of the current gerrit-download function. It uses the REST API for querying the open changes in the current project and uses magit for downloading the change.

(defun gerrit-format-change (change)
  (concat
   (propertize (number-to-string (alist-get '_number change)) 'face 'magit-hash)
   " "
   (propertize (alist-get 'branch change) 'face 'magit-branch-remote)
   " "
   (propertize (alist-get 'subject change) 'face 'magit-section-highlight)
   ))

(defun gerrit--get-refspec(change-metadata)
  ;; this is important for determining the refspec needed for
  ;; git-fetch
  ;; change-ref is e.g. "refs/changes/16/35216/2"
  (let* ((revisions (alist-get 'revisions change-metadata))
         (revision (alist-get 'current_revision change-metadata)))
    (gerrit--alist-get-recursive (intern revision) 'ref revisions)))

(defun gerrit--download-change (change-metadata)
  (let* ((change-nr (alist-get '_number change-metadata))
         (change-branch (alist-get 'branch change-metadata))
         (change-topic (or (alist-get 'topic change-metadata)
                           (number-as-string change-nr)))
         ;; TODO ensure that gerrit--acounts-alist is initialized
         (change-owner (alist-get (gerrit--alist-get-recursive
                                   'owner '_account_id change-metadata)
                                  gerrit--accounts-alist))
         (local-branch (format "reviews/%s/%s" change-owner change-topic)))

    ;; TODO log messages?

    ;; this runs async, which is not what I want, because then I can't
    ;; run other git commands afterwards
    ;; (magit-fetch-refspec (gerrit-get-remote) (gerrit--get-refspec change-metadata) nil)
    (magit-call-git "fetch" (gerrit-get-remote) (gerrit--get-refspec change-metadata))
    ;; TODO handle errors of magit-branch-and-checkout:
    ;; check if a local branch with this name already exists
    ;; if yes, determine the remote and remote-branch of this local-branch
    ;;         and reuse it only if they are equal with the current-remote
    ;;         (gerrit-get-remote)
    ;;              and `change-branch`
    (magit-branch-and-checkout local-branch "FETCH_HEAD")))

(defun gerrit-download-new ()
  "Download change from the gerrit server."
  (interactive)
  (let* ((open-changes
          (seq-map #'gerrit-format-change (gerrit-rest-change-query
                                    (concat "status:open project:"
                                            (gerrit-get-current-project)))))
         (selected-line (completing-read
                         "Download Change: " open-changes nil nil))
         (changenr (car (s-split " " (s-trim selected-line))))

         ;; the return value of `gerrit-rest-change-query` contains the
         ;; current revision, but not the one of `gerrit-rest-change-get`.
         (change-metadata (car (gerrit-rest-change-query changenr))))

    (gerrit--download-change change-metadata)))
twmr commented 3 years ago

The gitreviewless branch already contains a new upload and download functions that don't depend git-review. I hope to merge this branch in the next months.