[cfe-dev] Questions about Workflow & Submitting Patches

[Shyan Akmal via cfe-dev]

Ah alright, makes sense. Thanks again everyone for the help!

On Wed, Oct 17, 2018 at 1:43 AM Kristóf Umann <dkszelethus at gmail.com> wrote:

>
>
> On 17 Oct 2018 10:09, "Manuel Klimek via cfe-dev" <cfe-dev at lists.llvm.org>
> wrote:
>
> 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 :)
>
> As far as I know, folks working on clang-tidy prefer a single patch, but
> for the static analyzer smaller patches are preferred. But indeed they will
> let you know.
>
>
>>
>> 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
>>>>
>>>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181017/b2dc81c1/attachment.html>