[cfe-dev] Questions about Workflow & Submitting Patches

[Manuel Klimek via cfe-dev]

On Wed, Oct 17, 2018 at 1:41 AM Shyan Akmal <sakmal at g.hmc.edu> wrote:

> Thank you everyone for the suggestions! The details in the workflow
> suggested by Whisperity seem like they'll be very helpful to us moving
> forward.
>
> Manuel's point of keeping all reviews public makes a lot of sense. Are
> there any general guidelines on how large we can make a single patch? In
> general I know that keeping changes smaller is better, but is it fine to
> write an entirely new clang-tidy check, and then submit that as a single
> patch?
>

I think most clang-tidy patches come as a single patch. When the patch is
too large people will usually tell you to split it up :)


>
>
> On Tue, Oct 16, 2018 at 2:29 AM Manuel Klimek <klimek at google.com> wrote:
>
>> On Tue, Oct 16, 2018 at 4:14 AM Shyan Akmal <sakmal at g.hmc.edu> wrote:
>>
>>> Hello,
>>>
>>> I'm working with a team of students that's interested in contributing to
>>> the clang front end (with the initial step of writing new checks for
>>> clang-tidy).
>>>
>>> We'd like to setup a workflow that makes it easy for us to review each
>>> other's work before officially submitting changes upstream, and also will
>>> be amenable to officially integrating our changes to clang-tidy.
>>>
>>
>> My advice would be to not do that, but to do all reviews publicly in
>> phabricator. Otherwise the reviewer that accepts the patch will potentially
>> go into all the same questions again that were discussed earlier, and the
>> later you get architectural feedback, the more expensive to change it, so
>> the chance to get insight from folks as early as possible is a huge benefit.
>>
>> Generally, I'd want to teach students that one of the most important
>> rules of software development is that change is cheaper the earlier it is
>> made, so discovering what needs to change as early as possible is probably
>> the single most important skill one can develop early in one's career :)
>>
>>
>>> Our current plan is to fork the git mirror of LLVM, and make commits to
>>> this copy of the repo on GitHub. After we finish writing and reviewing a
>>> check, we'd get the diff and submit a patch through Phabricator.
>>>
>>> This seems inline with the process suggested in the LLVM docs (from
>>> https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-git-svn
>>> and https://llvm.org/docs/Phabricator.html).
>>>
>>> *Questions*:
>>>
>>>    - Does this workflow make sense, and is it in accordance with LLVM
>>>    developer policy?
>>>    - Is there an alternate (or more standard) workflow that anyone
>>>    thinks would work better?
>>>    - If we work through a git repo, is it necessary to use git-svn to
>>>    make sure svn (and not just git) has changes saved?
>>>    - Are there any specific practices we should adopt to make sure our
>>>    additions can be successfully integrated into the current version of LLVM?
>>>
>>> Best,
>>> Shyan
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181017/41eea2a4/attachment.html>