[llvm-dev] Phabricator down for maintenance tonight
Tom Stellard via llvm-dev
llvm-dev at lists.llvm.org
Tue Jul 28 16:35:16 PDT 2020
On 7/28/20 6:58 PM, Mehdi AMINI wrote:
> +Tom Stellard <mailto: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?
>
Sure, I've cloned this and added you as an admin for the team, so you
can grant commit access.
-Tom
> --
> Mehdi
>
>
>
> On Tue, Jul 28, 2020 at 12:49 PM MyDeveloper Day
> <mydeveloperday at gmail.com <mailto: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
> <http://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 <mailto: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
> <mailto:joker.eph at gmail.com>> wrote:
>
>
>
> On Tue, Jul 28, 2020 at 11:04 AM MyDeveloper Day
> <mydeveloperday at gmail.com <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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 <mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
More information about the llvm-dev
mailing list