D10797: Add asterisk when an annotation is associated to non-empty popup

[Henrik Fehlauer]

rkflx added a comment.


  In D10797#241886 <https://phabricator.kde.org/D10797#241886>, @simgunz wrote:
  
  > My doubt is: when you want to review a "review" what do you do exactly (I never did it). You download a patch and apply it manually to your code (probably arc does this with some command)?
  
  
  Anyone wanting to review or simply try out a patch usually just issues `arc patch D10797`. This will make sure the repo is up-to-date to be able to find the commit you attached your revision to, create a new local branch named `arcpatch-D10797`, download all dependent revisions and finally apply the patch. In case the author updates the patch, the reviewer will re-issue `arc patch`, get a new branch called `arcpatch-D10797_1` and so on. Note that on these branches there is only a single squashed commit per Diff, i.e. (at least currently, might change in the future) what you see in Revision Contents > Commits is only visible on Phab.
  
  To investigate what the author changed, you could compare both local branches, but I guess most people just just the options for comparing revisions under Revision Contents > History.
  
  > If I overwrite some commits, as I did today, does it create any problem to the reviewer that already "pulled" from arc the review once? (in git this will create diverging branches between local and remote) Or does arc redownload the diff completely each time, so avoiding problems?
  
  Phabricator remembers all Diffs you uploaded. `arc patch` will simply re-download the complete patch, using the latest revision unless you specify an older revision.
  
  > This was not my fear, but I feared to mess up things to the reviewer that already "pulled" the review before.
  
  Nothing to fear. If it looks good in the Diff viewer on Phabricator, it will look good for your reviewer.
  
  > It was complaining about the fact that the commit with the revert did not contain a valid "Differential Revision" ID
  
  Ah, my comment might have been misleading. This line will only get added once you do `arc patch` or `arc amend`. However, you won't need to issue those commands or even add the line manually, because Arcanist will do it for you once you `arc land`.
  
  There is a second way `arc diff` can recognize the revision, which works by comparing your local commit sha with what has been recorded on Phabricator (IIRC – if not, `arc which` will tell you the real reason). If you change things in such a way that the commit sha on your local branch changed (which should not happen if you just add a reverting commit), then you'd have to manually provide the revision ID.
  
  > I'll also have a look at  T7116 <https://phabricator.kde.org/T7116> and maybe I can give some feedback from a point of view of a newcomer.
  
  That would be appreciated ;) (Or just update the Wiki yourself.)

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D10797

To: simgunz, #okular, aacid
Cc: rkflx, aacid, ngraham, michaelweghorn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180407/55c03d8a/attachment-0001.html>