[PATCH] D128645: Update developer policy.

Dávid Bolvanský via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 08:27:58 PDT 2022


xbolva00 added inline comments.


================
Comment at: llvm/docs/DeveloperPolicy.rst:87
 
-#. Patches should be made with ``git format-patch``, or similar (see special
-   commands for `Requesting Phabricator review via the web interface
-   <Phabricator.html#phabricator-request-review-web>`_ ). If you use a
-   different tool, make sure it uses the ``diff -u`` format and that it
-   doesn't contain clutter which makes it hard to read.
-
-Once your patch is ready, submit it by emailing it to the appropriate project's
-commit mailing list (or commit it directly if applicable). Alternatively, some
-patches get sent to the project's development list or component of the LLVM bug
-tracker, but the commit list is the primary place for reviews and should
-generally be preferred.
-
-When sending a patch to a mailing list, it is a good idea to send it as an
-*attachment* to the message, not embedded into the text of the message.  This
-ensures that your mailer will not mangle the patch when it sends it (e.g. by
-making whitespace changes or by wrapping lines).
-
-*For Thunderbird users:* Before submitting a patch, please open *Preferences >
-Advanced > General > Config Editor*, find the key
-``mail.content_disposition_type``, and set its value to ``1``. Without this
-setting, Thunderbird sends your attachment using ``Content-Disposition: inline``
-rather than ``Content-Disposition: attachment``. Apple Mail gamely displays such
-a file inline, making it difficult to work with for reviewers using that
-program.
-
-When submitting patches, please do not add confidentiality or non-disclosure
-notices to the patches themselves.  These notices conflict with the LLVM
-licensing terms and may result in your contribution being excluded.
+#. Patches should be unified diffs with "infinte context" (i.e. using something
+   like `git diff -U 999999 main`).
----------------
aaron.ballman wrote:
> vext01 wrote:
> > aaron.ballman wrote:
> > > vext01 wrote:
> > > > aaron.ballman wrote:
> > > > > Changing this would require an RFC to see if the community wants to get rid of our requirement that patches be formatted. Personally, I'd be opposed to such a change; I think this should be kept.
> > > > Is there confusion between `git format-patch` and `git clang-format` here?
> > > > 
> > > > To be clear, I'm not proposing that the source code you change isn't syntactically formatted. But `git format-patch` does not do syntactic formatting, it just writes a diff to disk.
> > > > 
> > > > I don't think it matters how you generate your diff, but your changes need to have gone through `git clang-format` as described elsewhere in the llvm docs.
> > > Oh, yes, that is confusion on my end! I was thinking `git clang-format`, so this part is fine by me. Thanks!
> > Woah! I just realized that `-U 999999` and `-U999999` are interpreted differently by `git`. We need the latter.
> TIL I've been doing it right almost entirely by chance. :-)
> 
> You should probably fix up the patch summary as well.
typo "infinte"


================
Comment at: llvm/docs/DeveloperPolicy.rst:88
+#. Patches should be unified diffs with "infinte context" (i.e. using something
+   like `git diff -U 999999 main`).
+
----------------
main is weird example here imho.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128645/new/

https://reviews.llvm.org/D128645



More information about the llvm-commits mailing list