[llvm-dev] Phabricator down for maintenance tonight
Mehdi AMINI via llvm-dev
llvm-dev at lists.llvm.org
Tue Jul 28 15:58:05 PDT 2020
+Tom Stellard <tstellar at redhat.com> : can we get a fork of
https://github.com/phacility/phabricator in the LLVM project org on GitHub
in order to host our patched version and start collaboratively maintaining
it?
--
Mehdi
On Tue, Jul 28, 2020 at 12:49 PM MyDeveloper Day <mydeveloperday at gmail.com>
wrote:
>
> Could we ever consider adding
> https://github.com/r4nt/phabricator/tree/llvm-production as a new
> read/only observe Diffusion repository in reviews.llvm.org? I'd be happy
> to do code reviews?
>
> MyDeveloperDay
>
> On Tue, Jul 28, 2020 at 8:46 PM MyDeveloper Day <mydeveloperday at gmail.com>
> wrote:
>
>> Awesome, thanks for sharing
>>
>> Here is a patch (based off yours) but this adds the "Collapse" button
>> back in,
>>
>> remember to rerun phabricator/bin/celerity map
>>
>> MyDeveloperDay
>>
>> diff --git
>> a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
>> b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
>> index fdaa99a..9b18031 100644
>> --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
>> +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
>> @@ -214,6 +214,23 @@ final class PHUIDiffInlineCommentDetailView
>> (!$is_synthetic);
>>
>> if ($can_reply) {
>> + $action_buttons[] = id(new PHUIButtonView())
>> + ->setTag('a')
>> + ->setIcon('fa-reply')
>> + ->setTooltip(pht('Reply'))
>> + ->addSigil('differential-inline-reply')
>> + ->setMustCapture(true)
>> + ->setAuralLabel(pht('Reply'));
>> +
>> + $action_buttons[] = id(new PHUIButtonView())
>> + ->setTag('a')
>> + ->setIcon('fa-times')
>> + ->setTooltip(pht('Collapse'))
>> + ->addSigil('differential-inline-collapse')
>> + ->setMustCapture(true)
>> + ->setAuralLabel(pht('Collapse'));
>> +
>> +
>> $menu_items[] = array(
>> 'label' => pht('Reply to Comment'),
>> 'icon' => 'fa-reply',
>> diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js
>> b/webroot/rsrc/js/application/diff/DiffChangesetList.js
>> index 7293f89..bb84997 100644
>> --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
>> +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
>> @@ -2402,6 +2402,19 @@ JX.install('DiffChangesetList', {
>> ['differential-inline-comment', 'inline-action-dropdown'],
>> onmenu);
>>
>> + // restore button for replying to comments
>> + var onreply = JX.bind(this, this._onInlineEvent, 'reply');
>> + JX.Stratcom.listen(
>> + 'click',
>> + ['differential-inline-comment',
>> 'differential-inline-reply'],
>> + onreply);
>> + var oncollapse = JX.bind(this, this._onInlineEvent, 'collapse');
>> + JX.Stratcom.listen(
>> + 'click',
>> + ['differential-inline-comment',
>> 'differential-inline-collapse'],
>> + oncollapse);
>> + // END
>> +
>> var ondraft = JX.bind(this, this._onInlineEvent, 'draft');
>> JX.Stratcom.listen(
>> 'keydown',
>> @@ -2491,6 +2504,12 @@ JX.install('DiffChangesetList', {
>> case 'delete':
>> inline.delete(is_ref);
>> break;
>> + case 'reply':
>> + inline.reply();
>> + break;
>> + case 'collapse':
>> + inline.setCollapsed(!inline.isCollapsed());
>> + break;
>> case 'draft':
>> inline.triggerDraft();
>> break;
>>
>> On Tue, Jul 28, 2020 at 8:02 PM Mehdi AMINI <joker.eph at gmail.com> wrote:
>>
>>>
>>>
>>> On Tue, Jul 28, 2020 at 11:04 AM MyDeveloper Day <
>>> mydeveloperday at gmail.com> wrote:
>>>
>>>> Out of interest are you keeping the local modifications in a fork of
>>>> the Phabricator source (llvm-phabricator)? Firstly we should be keeping any
>>>> changes we make in source control but also it's good to review those
>>>> changes.
>>>>
>>>
>>> I didn't innovate, Manuel set it up originally and I just reused the
>>> flow there: https://github.com/r4nt/phabricator/tree/llvm-production
>>>
>>> That said our "pre-production" setup isn't super streamlined, I'd like
>>> to improve this in the future. Let me know if you have suggestions :)
>>>
>>>
>>>> I have found over the years that I've had to redo some of my local
>>>> modifications as @evan changes the underlying infrastructure, nothing major
>>>> but sometimes can cause a little downtime when doing an upgrade.
>>>>
>>>
>>> Yeah we had some merge conflicts as well, I reduced some of our diff
>>> with upstream in the process, I think we can reduce a bit more.
>>>
>>> What I did for this upgrade is that 10 days ago I:
>>>
>>> - duplicate the entire VM and the database on a new machine.
>>> - merged the latest stable version and went through the conflicts
>>> - upgraded the database (had to manually craft some SQL queries as some
>>> duplicated records were breaking newly added keys).
>>> - tested manually this cloned instance by opening fake reviews. Had to
>>> fix the mail config since they changed the way it is configured.
>>> - Had to patch some broken code in phab in the SendGrid interaction with
>>> respect to attachments:
>>> https://discourse.phabricator-community.org/t/sendgrid-mailer-metamta-differential-attach-patches-true-errors-with-the-attachment-type-cannot-contain-or-crlf-characters/4098
>>>
>>> Then when everything was working, I took down the production instance
>>> and repeated the migration steps (except fixing the merge conflicts and the
>>> patching that I had kept in git).
>>>
>>> --
>>> Mehdi
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>>
>>>> MyDeveloperDay
>>>>
>>>> On Tue, Jul 28, 2020 at 6:17 PM Mehdi AMINI via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Jul 28, 2020 at 4:25 AM James Henderson <
>>>>> jh7370.2008 at my.bristol.ac.uk> wrote:
>>>>>
>>>>>> Thanks for the work too!
>>>>>>
>>>>>> Not specifically a regression, but since the upgrade, I find it
>>>>>> annoying now that when I want to do something in relation to an inline
>>>>>> comment (collapse it, reply to it etc), I now have to click on a drop-down
>>>>>> menu in the comment header bar to select the option, whereas before the
>>>>>> icons were inline there before. Is this something that can be easily
>>>>>> addressed?
>>>>>>
>>>>>
>>>>> Seems like https://secure.phabricator.com/D21244
>>>>>
>>>>> <https://secure.phabricator.com/D21244>I patched back the reply
>>>>> button, collapse seems a bit more intrusive. That said the keyboard
>>>>> shortcuts are pretty nice: you can click on a comment to select it, `n` for
>>>>> selecting the next one, `p` the previous one, `q` to collapse, `r` to reply.
>>>>>
>>>>> --
>>>>> Mehdi
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> James
>>>>>>
>>>>>> On Tue, 28 Jul 2020 at 05:56, David Blaikie via llvm-dev <
>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>>> Thanks for the work!
>>>>>>>
>>>>>>> On Mon, Jul 27, 2020 at 9:53 PM Mehdi AMINI via llvm-dev
>>>>>>> <llvm-dev at lists.llvm.org> wrote:
>>>>>>> >
>>>>>>> > Hi,
>>>>>>> >
>>>>>>> > Phab is upgraded now. It was ~2 years since the last upgrade so we
>>>>>>> got a few features along the way.
>>>>>>> >
>>>>>>> > We may have regressed some aspects as well, I only tested the
>>>>>>> basic functionalities.
>>>>>>> > Let me know if you see anything behaving unexpectedly!
>>>>>>> >
>>>>>>> > --
>>>>>>> > Mehdi
>>>>>>> >
>>>>>>> >
>>>>>>> > On Mon, Jul 27, 2020 at 7:53 PM Mehdi AMINI <joker.eph at gmail.com>
>>>>>>> wrote:
>>>>>>> >>
>>>>>>> >> Hi,
>>>>>>> >>
>>>>>>> >> FYI, I'm taking Phabricator down for an upgrade right now.
>>>>>>> >>
>>>>>>> >> --
>>>>>>> >> Mehdi
>>>>>>> >>
>>>>>>> > _______________________________________________
>>>>>>> > LLVM Developers mailing list
>>>>>>> > llvm-dev at lists.llvm.org
>>>>>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>
>>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org
>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200728/52cd00a0/attachment.html>
More information about the llvm-dev
mailing list