This is an archive of the discontinued LLVM Phabricator instance.

docs: Document workaround for arcanist failures
ClosedPublic

Authored by tstellar on Oct 1 2021, 2:45 PM.

Download Raw Diff

Details

Reviewers

smeenai
ctetreau

Commits

rG509fe20fbca7: docs: Document workaround for arcanist failures

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

tstellar requested review of this revision.Oct 1 2021, 2:45 PM

tstellar created this revision.

Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2021, 2:45 PM

Harbormaster completed remote builds in B126749: Diff 376641.Oct 1 2021, 2:45 PM

LGTM, thanks.

This revision is now accepted and ready to land.Oct 1 2021, 2:49 PM

Would we want to put up the commit in a fork that we control, so that we're not relying on someone else's fork (which may go away in the future)?

In D110976#3037605, @smeenai wrote:

Would we want to put up the commit in a fork that we control, so that we're not relying on someone else's fork (which may go away in the future)?

Ideally, a fork could live in the LLVM github org with this patch applied.

We discussed this at the last board meeting and decided that we don't want to be hosting non-LLVM projects in our GitHub organization. We can always update the docs if this workaround stops working.

In D110976#3046911, @tstellar wrote:

We discussed this at the last board meeting and decided that we don't want to be hosting non-LLVM projects in our GitHub organization. We can always update the docs if this workaround stops working.

This is weird, we already decided to host the repo for our own fork of Phabricator: https://github.com/llvm/phabricator

In D110976#3047077, @mehdi_amini wrote:

In D110976#3046911, @tstellar wrote:

We discussed this at the last board meeting and decided that we don't want to be hosting non-LLVM projects in our GitHub organization. We can always update the docs if this workaround stops working.

This is weird, we already decided to host the repo for our own fork of Phabricator: https://github.com/llvm/phabricator

I don't think anyone was aware of this, I will raise the question again.

In the meantime, I'm going to commit this so that we have a documented solution.

ctetreau added a reviewer: ctetreau.Oct 13 2021, 9:44 AM

Any news on this? As @mehdi_amini mentions, we have a fork. I would think that the ideal solution is to just merge that PR into our fork, then update the instructions to say to use our repo for arcanist. If our fork of phabricator is not long for this world, I don't think this would make getting rid of it in the future significantly harder.

Did you get any clarification on this issue @tstellar?

Closed by commit rG509fe20fbca7: docs: Document workaround for arcanist failures (authored by tstellar, committed by mehdi_amini). · Explain WhyOct 13 2021, 8:26 PM

This revision was automatically updated to reflect the committed changes.

mehdi_amini added a commit: rG509fe20fbca7: docs: Document workaround for arcanist failures.

In the meantime, I'm going to commit this so that we have a documented solution.

I pushed this in 509fe20fbca767a65b9c27750ea9d372ed2ab05a

In D110976#3061904, @ctetreau wrote:

Any news on this? As @mehdi_amini mentions, we have a fork.

Sorry: we only have a fork for the server side Phabricator software. We'd have to fork the arcanist repo as well.

OK, thanks for the update!

MaskRay mentioned this in D129255: [docs] Move code contribution from GettingStarted.rst to Contributing.rst.Jul 7 2022, 12:41 AM

MaskRay mentioned this in rG472aa7e6bbc3: [docs] Move code contribution from GettingStarted.rst to Contributing.rst.Jul 7 2022, 10:51 AM

Revision Contents

Path

Size

llvm/

docs/

GettingStarted.rst

12 lines

Diff 379599

llvm/docs/GettingStarted.rst

	Show First 20 Lines • Show All 461 Lines • ▼ Show 20 Lines
	<phabricator-reviews>`.			<phabricator-reviews>`.

	You'll generally want to make sure your branch has a single commit,			You'll generally want to make sure your branch has a single commit,
	corresponding to the review you wish to send, up-to-date with the upstream			corresponding to the review you wish to send, up-to-date with the upstream
	``origin/main`` branch, and doesn't contain merges. Once you have that, you			``origin/main`` branch, and doesn't contain merges. Once you have that, you
	can start `a Phabricator review <Phabricator.html>`_ (or use ``git show`` or			can start `a Phabricator review <Phabricator.html>`_ (or use ``git show`` or
	``git format-patch`` to output the diff, and attach it to an email message).			``git format-patch`` to output the diff, and attach it to an email message).

	However, using the "Arcanist" tool is often easier. After `installing			However, using the "Arcanist" tool is often easier. After `installing arcanist`_, you
	arcanist`_, you can upload the latest commit using:			will also need to apply a fix to your arcanist repo in order to submit a patch:

				.. code-block:: console

				% cd arcanist
				% git fetch https://github.com/rashkov/arcanist update_cacerts
				% git cherry-pick e3659d43d8911e91739f3b0c5935598bceb859aa

				Once this is all done, you can upload the latest commit using:

	.. code-block:: console			.. code-block:: console

	% arc diff HEAD~1			% arc diff HEAD~1

	Additionally, before sending a patch for review, please also try to ensure it's			Additionally, before sending a patch for review, please also try to ensure it's
	formatted properly. We use ``clang-format`` for this, which has git integration			formatted properly. We use ``clang-format`` for this, which has git integration
	through the ``git-clang-format`` script. On some systems, it may already be			through the ``git-clang-format`` script. On some systems, it may already be
	▲ Show 20 Lines • Show All 758 Lines • Show Last 20 Lines