[Commented On] D8524: phabricator: add .arcconfig to help messages and comments (issue6331)

[sfink (Steve Fink)]

sfink added a comment.


  In D8524#134322 <https://phab.mercurial-scm.org/D8524#134322>, @mharbison72 wrote:
  
  > The documentation updates look good to me, but there are still some code changes that were probably not intentional, and the commit message probably needs an update.
  
  I updated the commit message to "add .arcconfig to help messages and comments", which seems accurate.
  
  >> It gets a little messy when uploading partial patches to code that I am patching
  >
  > Are you simply amending your local commit and reposting it with phabsend?  If not, I'm not sure what the state of it will be if someone imports it to land it.
  
  Sorry, I said that wrong. Yes, I'm doing exactly what you're saying, it's just that the phabsend I'm using is coming from `mozphabricator.py` and I have a whole long stack of patches applied to it, and a corresponding stack of patches applied to `phabricator.py`, and I have to be sure to be in matching places in the stacks or things might get a little mangled (eg adding "Bug None - Bug None - Bug None -" to the beginning of the commit message).
  
  >> Ok, the problem I was running into with the exthelper stuff is that it wasn't getting invoked, and it wasn't getting invoked because my extension did not set its uisetup at all. So the fix is very similar to the above: from hgext.phabricator import uisetup
  >
  > I've got a similar extension that wraps the command itself and changes all reviewers to blocking reviewers (so that the first accept doesn't drop it out of everybody else's queue).  I don't understand importing `hgext.phabricator.uisetup` though.  You should have this somewhere near the top of your extension like `phabricator.py` does
  >
  >   eh = exthelper.exthelper()
  >   ...
  >   uisetup = eh.finaluisetup
  >
  > The underlying machinery simply looks for attributes on the module like `uisetup()`, `reposetup()`, etc, and calls them if present.  And yeah, I get confused all the time about the differences between these hooks too.
  
  Hm, you may be right. I hadn't needed `exthelper` for anything else in my wrapper yet, so I got rid of it entirely. (I was originally using it, but that meant when I did eg `commands = eh.commands` I wasn't "inheriting" the commands from `phabricator.py`. So I switched to `from hgext.phabricator import commands` and then modifying stuff as needed.)
  
  Anyway, whether I do `uisetup = eh.finaluisetup` or `from hgext.phabricator import uisetup` doesn't matter too much; it ends up having the same effect. Given that `phabricator.py` already has an `exthelper` object instantiated with configuration on it that I want to use, it seems a little odd to make another one in my module. Perhaps a midway point would be `from hgext.phabricator import eh` and then `commands = eh.commands` etc.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8524/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8524

To: sfink, mharbison72, marmoute
Cc: marmoute, Kwan, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200829/613bf20c/attachment-0002.html>