I don't think I'll have capacity to review every PR that brings particular Beam transforms in accordance with the style guide - but I'm happy to review some of them and participate in discussions of potentially more controversial changes. In my ideal world, this task would be crowdsourced (I sent out a separate thread about that).
On Mon, Jan 30, 2017 at 5:43 PM Jesse Anderson <[email protected]> wrote: > Thanks for putting that together. Does this mean you've volunteered to > referee bikeshedding? > > On Mon, Jan 30, 2017 at 5:21 PM Eugene Kirpichov > <[email protected]> wrote: > > > The initial PR has been merged and the style guide is live > > https://beam.apache.org/contribute/ptransform-style-guide/ - let us > > continue discussing and tweaking on this thread and via smaller PRs > > modifying the document. > > > > On Mon, Jan 30, 2017 at 7:50 AM Aljoscha Krettek <[email protected]> > > wrote: > > > > > Wow, that's a long read. But quite informative +1 > > > > > > On Sat, 28 Jan 2017 at 06:54 Jean-Baptiste Onofré <[email protected]> > > wrote: > > > > > > > Hi Eugene, > > > > > > > > As said in the PR: great work and thanks a lot ! > > > > > > > > I will take a complete look during the week end. I'm pretty sure > it's a > > > > great guide as it's basically the result of our discussions and > reviews > > > ;) > > > > > > > > Thanks again ! > > > > Regards > > > > JB > > > > > > > > On 01/28/2017 06:21 AM, Eugene Kirpichov wrote: > > > > > Hello all, > > > > > > > > > > I just sent a pull request with a style guide for developers of new > > > > > PTransforms - intended for library writers, e.g. people who > > contribute > > > > new > > > > > connectors and other transforms to Beam. The guide is mainly based > on > > > > > experience from reviewing connectors contributed by JB and others, > > but > > > > it's > > > > > intended to be generally applicable. > > > > > > > > > > It covers a variety of points - from code organization, to overall > > API > > > > > design, to error handling and so on. I expect most of it to be > > > > > non-controversial and just reflect the style of existing transforms > > in > > > > Beam > > > > > - however all of it is, of course, up to debate. > > > > > > > > > > https://github.com/apache/beam-site/pull/134/ > > > > > > > > > > I'm hoping that this documentation will help guide new transform > > > authors > > > > in > > > > > the right direction from the start, as well as make the job of > > > reviewers > > > > > easier by providing a source they can link to and helping focus the > > > > review > > > > > on resolving more ambiguous points. > > > > > > > > > > (Note that, like all other documentation, this will evolve, so the > > goal > > > > of > > > > > the current PR is not to be complete, but to be a starting point) > > > > > > > > > > When the guide is ratified, I think it'll make sense to file JIRAs > to > > > > bring > > > > > Beam in accordance with it - there are a few transforms that were > > > written > > > > > before the best practices shaped up. > > > > > > > > > > Thanks! > > > > > > > > > > > > > -- > > > > Jean-Baptiste Onofré > > > > [email protected] > > > > http://blog.nanthrax.net > > > > Talend - http://www.talend.com > > > > > > > > > >
