<div dir="ltr">Ah alright, makes sense. Thanks again everyone for the help!</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Oct 17, 2018 at 1:43 AM Kristóf Umann <<a href="mailto:dkszelethus@gmail.com">dkszelethus@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><br><div class="gmail_extra" dir="auto"><br><div class="gmail_quote">On 17 Oct 2018 10:09, "Manuel Klimek via cfe-dev" <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="m_-6302943702762512244quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div class="m_-6302943702762512244quoted-text"><div dir="ltr">On Wed, Oct 17, 2018 at 1:41 AM Shyan Akmal <<a href="mailto:sakmal@g.hmc.edu" target="_blank">sakmal@g.hmc.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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. <div><br></div><div>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? <br></div></div></blockquote><div><br></div></div><div>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 :)</div></div></div></blockquote></div></div><div dir="auto">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.</div><div class="gmail_extra" dir="auto"><div class="gmail_quote"><blockquote class="m_-6302943702762512244quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div class="m_-6302943702762512244quoted-text"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 16, 2018 at 2:29 AM Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Oct 16, 2018 at 4:14 AM Shyan Akmal <<a href="mailto:sakmal@g.hmc.edu" target="_blank">sakmal@g.hmc.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hello,<div><br></div><div>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). </div><div><br></div><div>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.</div></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>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 :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>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. </div><div><br></div><div>This seems inline with the process suggested in the LLVM docs (from <a href="https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-git-svn" target="_blank">https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-git-svn</a> and <a href="https://llvm.org/docs/Phabricator.html" target="_blank">https://llvm.org/docs/Phabricator.html</a>). </div><div><br></div><div><b>Questions</b>: </div><div><ul><li>Does this workflow make sense, and is it in accordance with LLVM developer policy?</li><li>Is there an alternate (or more standard) workflow that anyone thinks would work better?</li><li>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? </li><li>Are there any specific practices we should adopt to make sure our additions can be successfully integrated into the current version of LLVM?</li></ul><div>Best,</div></div><div>Shyan</div></div></div></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div></div></div>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div>