twmr / gerrit.el

gerrit integration in emacs
43 stars 6 forks source link

feature request: allow new arguments to be added to upload transient #45

Closed rmmanseau closed 2 years ago

rmmanseau commented 2 years ago

https://github.com/thisch/gerrit.el/blob/4de561d1295d4c86ca9b159ab0c746bedc2d0380/gerrit.el#L433-L434

the above line in the condition of gerrit-upload:--action causes the function to error out if there are unknown transient args rather than just ignoring them. I tried adding an argument to the upload transient that would enable an env var before calling the function and then disable it afterwards, but it ended up not working because the function would error out at the above lines when it got around to parsing the new argument I injected into the transient. See my snippets of code here

;;
;; add advice that enables skipping the pre push hook by setting an env var before
;; calling the upload function and then unsetting it after
;;
(defadvice gerrit-upload:--action (around advice-gerrit-upload:--action activate)
  (interactive)
  (if (called-interactively-p 'any)
      (progn
        (cl-loop for arg in (transient-args 'gerrit-upload-transient) do
                 (cond ((string= "skip-checks" arg)
                        (setenv "GIT_SKIP_PRE_PUSH_HOOK" "1"))))
        (call-interactively (ad-get-orig-definition 'gerrit-upload:--action))
        (setenv "GIT_SKIP_PRE_PUSH_HOOK" nil))
    ad-do-it))

(transient-insert-suffix 'gerrit-upload-transient (list 0 -1)
  '("s" "Skip checks" "skip-checks"))

I ended up overriding gerrit-upload:--action and deleting the t condition but it would be nice if that change were upstreamed into this repo so that I could implement this only using advice.

twmr commented 2 years ago

@rmmanseau thx for creating this issue report. I'll upload a fix tomorrow.

twmr commented 2 years ago

BTW I think we can add proper support for skipping pre-push checks by adding the --no-verify flag, which is also available in git-commit (for skipping pre-commit hooks) and git-push, to the transient.

https://stackoverflow.com/questions/29545972/skip-githooks-on-git-push

Although it was not mentioned, --no-verify also works on git push :)

rmmanseau commented 2 years ago

thanks!!