Hi Allen,

Thanks for your valuable comments, (BTW your new email address is
automatically categorized to spam by Gmail. Gmail noticed the email address
cannot be verified.)

Please see my responses inline:

On Tue, Sep 20, 2016 at 2:56 PM, Allen Wittenauer <a...@effectivemachines.com>

> On 2016-09-09 15:54 (-0700), Wangda Tan <wheele...@gmail.com> wrote:
> > We propose to merge YARN-3368 (YARN next generation web UI) development
> > branch into trunk for better development, would like to hear your
> thoughts
> > before sending out vote mail.
> >
> A quick pass through the diff:
> * 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.

For example .editorconfig is used for make code style consistent across
different IDEs/envs, which is pretty important for the code style.

And .watchmanconfig is used to do monitoring code changes while doing

Personally they're not redundant files to me and they can make development
environment can be setup easier.

If you have any specific files want to remove from the source tree, I can
double check it.

> * versions of things be set in a higher order pom rather than children if
> possible
> * RAT exclusions for files where the license header can be added and files
> that don't actually exist
> * documentation references deprecated variables
> * documentation top and bottom are basically the same information

Will check above 4 items.

> * why isn't configuration in HADOOP_CONF_DIR?  That seems like a major
> blocking issue, especially from a downstream packaging front.  users are
> never going to find that because we've trained them to look in H_C_D.

Yeah I agree it will be better to put configs into HADOOP_CONF_DIR,
YARN-5145 is originally opened for this, we have some discussions
on YARN-5145 for this, it seems there's no easy solution for this.

We will try to see if this can be done in a reasonable way.

> * 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.

>     * 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?

> * 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.

> * for an optional component, why is it first in the main pom.xml's module
> list?  shouldn't it be last to verify that the other stuff gets built first
> and therefore truly optional?

Will update

> * isn't this bundling the test code together with the actual application
> code?  why? also, is there any concern about the test code being used as a
> backdoor?

Will look into if there's any security concern for this. And will remove
test code from packaging if it is not necessary.

> * filenames with spaces and mixed case are going to cause all sorts of
> unexpected problems

If you're taking about "Sorting icons.psd", this part of the dependency of
datatables, will check if it possible to modify it, but since datatables is
wildly used, I think it should be safe in general.

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: yarn-dev-unsubscr...@hadoop.apache.org
> For additional commands, e-mail: yarn-dev-h...@hadoop.apache.org

Reply via email to