> On Sep 22, 2016, at 11:13 AM, Wangda Tan <wheele...@gmail.com> wrote:
> >* why are there editor settings and other superfluous things there? do
> >these settings comply with the PMC's style guide? why would they be here
> >and not in the base of the source tree?
> Actually many front-end projects are include these files in their source
> tree. I think the major reason is front end development needs different
> toolchains comparing to Java development.
That's fine for those projects, but we're talking about this
project--Apache Hadoop. We strive (and even sometimes achieve) code consistency
throughout the entire project. It's so important that it's even codified in
the committer guidelines. Why should one chunk of code--in a sub-project, and
a currently optional component at that--have settings different (?) than the
rest of the code base? Having potentially different formatting rules is a
direct hinderance to others looking at and contributing to the code.
In order to respect the guidelines established by the PMC, it seems
these either need to get moved to the root of the source tree and/or made
consistent with the rest of the code or deleted.
> >* I'm still not sure why this is being wrapped in a maven profile:
> > * create-release is activating the profile, so why not enable it for
> > everyone?
>
> The major reason is, create-release run using docker image, all necessary
> dependencies are already included. But we don't expect every developer
> install npm and all the other dependencies to build the UI code.
>
> This is similar to native code, user who is interested to modify and
> repackage it can enable the profile and build, but I don't expect everybody
> is required to do that.
This is a good point. However, if the intent is to make this the
default UI, then it's better to bite the bullet and put these requirements in
place now while 3.x is in alpha rather than flip the switch later.
> > * precommit won't test it with that profile in place; it'd be useful to
> see a *real* run of yetus against this branch without the maven profile
> protecting it
>
> Actually this is also what I want to do, I'm not quite sure what need to do
> to enable profile while running the precommit, if it is possible, could you
> point me which code I need to update?
I grouped these two bullet points intentionally: with the exception of
some documentation bits, create-release is specifically coded currently to
build what precommit tests. (See, for example, ISA-L support). Turning this on
for create-release without precommit support is premature. You need to get a
Yetus issue filed to turn on that profile first.
It's worthwhile pointing out that without this support in place, this
is basically a discussion about merging a branch that has very little precommit
testing in place specifically because of the maven profile.
> >* this doesn't appear to actually build in target/, which is very counter to
> >maven. is there a valid reason for that?
>
> I will double check it, I just found bower supports to build in a separate
> folder, but not quite sure about npm. The bottom line is we should be able to
> copy the whole folder to target and build it.
Awesome. I recognize this is a pain, but maven really really really
really really doesn't like things outside of target/. We've got some other
things like this to fix, but we really need to work on making more of them.
Thanks.
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-dev-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-dev-h...@hadoop.apache.org
