Re: OFBiz UI work

[David E. Jones]

-David


On Jan 15, 2007, at 9:44 PM, Chris Howe wrote:

Ummm... what do you think when you think of a rejection policy? We already have patch rejection policies when problems are found...

I agree.  I think it's better that we strongly
encourage certain practices (as we are doing now) and
bring people to notice when those practices could be
improved, but rejection policies put your _means
perspective ahead of your _ends.  Contributions may be
easier to review, but I can gaurantee you with
rejection policies, you will then have less of it to
review and thereby less solutions making it back into
the project.

--- "David E. Jones" <[EMAIL PROTECTED]> wrote:

Yes, we want more people, but I don't think anyone
wants
indiscriminate changes going into SVN! I hate
surprises when I check
out as much as the next guy, probably more than the
next guy in many
cases.

Thinking about the next guy is the real key here.
You may want to get
your patches in ASAP, but if your patch breaks
existing code (for
example), then that needs to be fixed before the
commit is done
(either by the committer, the contributor, or an
interested third
party).

So, yeah, that slows things down and is
inconvenient, but keep in
mind that a large portion of patches that come into
OFBiz break
existing functionality. This seems to happen around
once a week or
so, at least.

It's great that people want to contribute, but in or
to contribute to
something as large and complex as OFBiz a large
amount of effort is
necessary, and anyone that wants to help out will
err on the side of
extra effort, caution and review, and consideration
of more general
requirements and designs rather just one's own
requirements.

-David



On Jan 15, 2007, at 7:42 PM, Jonathon -- Improov
wrote:

Heh. True. I definitely want MORE people involved

in OFBiz.

But since I'm not a committer, I'd rather make

things easier for

the committers. I have no control over how easy or

difficult it is

to handle patches with "extra unintended

footprints".

If I were a committer, I would try to

automatically filter out

formatting changes in my audit, and then INCLUDE

the formatting

changes. After all, there's no harm removing some

extra spaces at

the end of lines, part of clean up.

We often try to make things easier for the person

who is doing a

task that we have no control over. Eg, I'd keep my

mouth really

wide open for my dentist just so his vision is

20/20, no arguments

from me; I could afford to be slack about this

"mouth wide-open

policy" if I were able to do the dentistry myself.

But you're certainly right that it'll exclude some

people,

especially folks who use editors that do not give

very much control

to users.

Jonathon

Chris Howe wrote:

I don't know that rejection policies are very
community friendly.  Treating SVN code changes

like

the keeper of the Bridge of Death (Monty Python
Reference, smile) may find less people willing to

do

in this do-ocracy.  Many of us rather like what's

left

of our anarco-sydicalist commune (oh look,

there's

another :-) ).
--- Jonathon -- Improov <[EMAIL PROTECTED]> wrote:

David,

I agree with the rejection policy.

One suggestion on procedure to take when
self-reviewing a patch before submitting it.

Look at

the diff to see if there are changes we cannot

account

for. Using KDiff also makes things easier, even

when dealing with

formatting changes.

In Emacs, it's also easy to go through every set

of

changes, do an interactive-search for 1st entry

of each set, and

see if the 2nd entry is
highlighted similarly. Meaning, if it is

highlighted similarly,

the set of changes is bogus
(formatting only).

In general, we should submit patches that are
intentional, ie just the intended changes only.

We should not

submit patches that contain unintended
changes that have extra unintended footprints.

Jonathon

David E. Jones wrote:

Moving this back to the mailing list...


On Jan 15, 2007, at 5:34 PM, Adrian Crum wrote:

David E. Jones wrote:

When reviewing a patch lines with  formatting

changes only are

EXTREMELY time consuming, and the patch

attached that issue is a

good example of a painful one. For each of

those line you have to

stare at it looking at every character trying

to figure out what the

point of the bloody change was, or to make

sure

that the change is

"invisible"...

That really puzzles me. You keep mentioning

formatting changes when I

don't see any. I didn't make any formatting

changes when modifying the

files. Even now when I look at the patch in

Jira,

it shows only the

lines I changed in the file.

*shrug*

As others may have the same question I'll

answer

it on the mailing list.

Below is the first section from the

framework_v2.patch file on the

[https://issues.apache.org/jira/browse/OFBIZ-605]

issue.

Each change is marked with a "-" for a line

removed or a "+" for a line

added.

First Set: formatting, or "invisible" change

Second Set: add comment (not necessary, BTW as

it

describes something

that no longer exists; if people want to see

old

stuff they can look at

the SVN history); remove tabstyles.css link tag

Third Set: formatting, or "invisible" change

Of all of these changes, only the "remove

tabstyles.css link tag" was

actually intended and necessary, but getting to

this fact when reviewing

the changes takes some time... this making it

more

difficult to review,

commit, etc.

People tend to slip things into patches

accidentally all the time. I'm

tempted to begun the voting process for a new

policy that says that if

there is anything in the patch file not

described

in the summary of the

patch, then it will be rejected...

-David


========================
Index: common/webcommon/includes/header.ftl

=== message truncated ===

smime.p7s
Description: S/MIME cryptographic signature