Mailing-List: contact dev-help@reef.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@reef.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <BL2PR03MB290BF331E0DF707AD1E156BD2CD0@BL2PR03MB290.namprd03.prod.outlook.com>
References: 
 <CAPsjEAsbW2yxX-2Zb-VeAjW5Kt5TX+4jLiSD1CrD33xzd5RkAw@mail.gmail.com>
 <569855A6.9060404@weimo.de>
 <BL2PR03MB290BF331E0DF707AD1E156BD2CD0@BL2PR03MB290.namprd03.prod.outlook.com>
From: Anupam <anupam128@gmail.com>
Date: Tue, 19 Jan 2016 01:19:38 -0800
Message-ID: 
 <CAPsjEAv8LABZwqKchiVRJPYmK-byuVi1PP9x5YxAkjSVExi5fg@mail.gmail.com>
Subject: Re: Improving dev experience for REEF .NET
To: dev@reef.apache.org
Content-Type: multipart/alternative; boundary=001a1142201a936b250529ac5ff4

--001a1142201a936b250529ac5ff4
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Thanks everyone for replying.

Few things I would like to respond to and add.

"What should we do beyond annotating that code with `[Unstable]`?"
Mix of [Unstable] and "stable" APIs in the same assembly and namespace are
hard to work with for both REEF developers and users. As a user, Visual
Studio is not giving me intellisense on what I am not supposed to use. I
feel that developing APIs in internal REEF assembly and then graduating the
APIs over time to the public SDK assembly would be a better way of handling
this. As long as the user compiles their code against only SDK assembly,
they will be fine. (making this change cleanly without making a breaking
change would be a difficult thing to accomplish as you pointed out)

RTC vs CTR
I had collected these items over months and now I think 5.a is much more of
and issue than 5.b. I have usually had good experience with PRs wrt 5.b
since I have been working on REEF-70, REEF-227 and related JIRAs. If
anything, on a spectrum of RTC to CTR I would like to see more rigorous RTC
than we do now.

"This will be tricky to enforce, as we have plenty of "trivial" code
changes flying by."
Except for typos/cosmetic changes, there should either be a unittest added
or changed at the very least. We are not an old enough codebase to justify
non-addition of tests as the rigidity existing design. If we follow good
practices while writing the code in the first place, adding/changing
unittests should be extremely cheap (in some places that is not the case,
which clearly shows in the interfaces which are strongly coupled). We would
leave it to the collective wisdom of the contributor and code reviewer to
justify and accept no addition of unittest. In any case it should exception
and not the norm. We could also keep track of code coverage, with a grain
of salt.

"we somehow need to agree among the committers on review standards"
Please take a look at
https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark#Con=
tributingtoSpark-CodeReviewCriteria
or https://wiki.apache.org/hadoop/CodeReviewChecklist. We could begin from
something simple like these.

"So far, the coding guidelines are a straight copy from CoreFx, with the
added emphasis on immutability in REEF."
Personally I haven't seen much being followed except the immutability part
:). I doubt that all contributors and committers have gone through the
CoreFx guidelines, as they are too verbose and much of it does not apply to
us. Which brings me to the next point.

"Please have a look at our contributors guide and improve as you see fit"
Will do. Will basically borrow heavily from publicly available PR
guidelines (Hadoop, CoreFx, Spark etc) and send a draft. Will need some
help on the Java specific details.

+1 on generating nice website documentation. It also makes it easier to
point out problems with documentation.

I will be filing JIRA on few of the actionable items here.

@bgchun: as engineers we all just see (never ending?) room for
improvement... It is fun to contribute on REEF .NET :)

Please don't consider this as a wrap up of this thread. Feel free to add
more comments and more issues that you have noticed.

Thanks,

On 15 January 2016 at 12:27, Mariia Mykhailova <mamykhai@microsoft.com>
wrote:

> A couple of comments:
>
> >> b. Documentation for new features. What should be the process?
> > However, that documentation is currently largely in vain, as we haven't
> found a way to generate a nice website from it like we do on the Java sid=
e.
>
> REEF-113, easy to do if we're willing to use external tools for site
> generation. I've just tried to configure Doxygen (
> http://www.stack.nl/~dimitri/doxygen/, free open-source), and it produces
> a very similar site with very little configuration effort. I have several
> questions about appearance of the website, will ask in JIRA.
>
> >> 3. Build flakiness 4. Test coverage
>
> Yes, we need to put more emphasis on test quality.
> Right now we have two ongoing issues with existing tests, REEF-1079 which
> I'm looking at, and REEF-1040 which is pretty much abandoned.
>
> -Mariia
>
>
> -----Original Message-----
> From: Markus Weimer [mailto:markus@weimo.de]
> Sent: Thursday, January 14, 2016 6:13 PM
> To: dev@reef.apache.org
> Subject: Re: Improving dev experience for REEF .NET
>
> thanks for putting these together!
>
>
> On 2016-01-14 17:17, Anupam wrote:
> > 1. Stable APIs a. Tagging new API which could change frequently b.
> > API spread too thin across assemblies c. Accidentally public APIs and
> > their deprecation
>
> We are acting on those, right? Once REEF-1087 is completed, we should be
> in much better shape in terms of making only the real APIs `public`.
> That in turn should enable us to act on the assembly structure.
>
> > b. Documentation for new features. What should be the process?
>
> There are two levels of documentation: API docs and prose on that what an=
d
> why of a new feature. I think we should absolutely have API docs for ever=
y
> `public` interface, class and method. However, that documentation is
> currently largely in vain, as we haven't found a way to generate a nice
> website from it like we do on the Java side.
>
> Regarding tutorials and such, we are doing badly. Some work has been done=
,
> but this clearly can be improved. Not sure how to make the time for that,
> though. Everybody agrees that we *should* do something, but feature work
> seems to get in the way of that pretty reliably.
>
> > d. Example code should be exemplary 3. Build flakiness 4. Test
> > coverage
>
> +1 on each of them.
>
> All of these are "easy" fixes. We know how this should be done, we just
> haven't.
>
> > 5. Pull request process
>
> This is the thorny one. Like most Big Data projects in Apache, we've
> decided on a review-then-commit (RTC) workflow, which means every
> contribution needs to somehow motivate a committer to review it.
> Further, we somehow need to agree amongst the committers on review
> standards. Both of these `somehow` statements are extremely tricky in
> practice.
>
> > b. (Subjective) Lack of process for unstable APIs along with PR
> > turn-around time (sometimes) makes me wary of splitting work into
> > small PRs.
>
> What should we do beyond annotating that code with `[Unstable]`?
>
> PR turn-around time will always be dependent on the load on the
> committers, unless we decide to ditch RTC for CTR. If we do that, the wor=
k
> of the release manager becomes more substantial: They'd have to orchestra=
te
> several rounds of cleanups before the release can be cut.
> Other projects do that, so it isn't out of the question.
>
> > Make a separate project named something like
> > Org.Apache.REEF.SDK/Org.Apache.REEF.APIs which has all the supported
> > stable and public APIs. [...] Consumption: Split NuGets into SDK and
> > Runtime (absolute minimum to run a REEF app) packages. IMRU and such
> > applications can have their own NuGets.
>
> +1, with one change: We need one each for REEF, Wake, Tang.
>
> One issue I see with this is that we either have to rename all types agai=
n
> or have to have projects where the DLL, NuGet and default namespace all
> have different names.
>
> > Build flakiness: Use IDE features as much as possible. Minimize
> > manually changing build files as much as possible. Clean up
> > build.props to not have Targets but just have aliases. Remove hidden
> > common operations done via build.props across multiple project builds,
> > this makes us susceptible to build concurrency issues.
>
> +1, this is a bug and really shouldn't need a list discussion to drive
> consensus. Feel free to file JIRAs.
>
> > Test coverage: Mandating addition of unittests for all new code added.
> > Add E2E tests, run them on localruntime as well as one box setup of
> > YARN (Mesos?). We need to add stress tests.
>
> [REEF-111] captures the infrastructure aspects of this. I believe Mariia
> is looking into it. Regarding the mandate for new tests: This will be
> tricky to enforce, as we have plenty of "trivial" code changes flying by.
> Asking for tests in addition to those would likely slow us down. For new
> features, I am all with you. Which brings us back to the question of how =
to
> set common expectations for PR reviews.
>
> > Pull request: We need to come up with PR guidelines
>
> Please have a look at our contributors guide and improve as you see fit:
>
>
> https://na01.safelinks.protection.outlook.com/?url=3Dhttps%3a%2f%2fcwiki.=
apache.org%2fconfluence%2fdisplay%2fREEF%2fContributing&data=3D01%7c01%7cma=
mykhai%40microsoft.com%7c511e60c6254e4edf2b9908d31d51680f%7c72f988bf86f141a=
f91ab2d7cd011db47%7c1&sdata=3De4UKAdvf4%2fWoRe4Xagl2DXeAkuf24L2YRKf2K%2fKyq=
kE%3d
>
> So far, the coding guidelines are a straight copy from CoreFx, with the
> added emphasis on immutability in REEF.
>
> Markus
>


--=20
Anupam
Bellevue, WA
Ph: +1 (425)-777-5570

--001a1142201a936b250529ac5ff4--