POLICY - Patch Acceptance
Ilias Lazaridis
code base instead of writing customized code, a patch acceptance and
application policy should be provided.
This is mainly to avoid rejection of patches, e.g. due to personal
reasons, non-understanding reasons (e.g. due to missing time) etc.!
A patch from a contributor A could be rejected, but the patch would be
of benefit for the reusability of the trac-code-base, and thus for all
other users.
A example rule of such policy could be:
* A patch which makes a function general, thus it can be used from
outside of trac-application
* should be applied *immediately*, if the existent behaviour is not
broken.
-
An example:
This patch would pass without *any* discussion, as it enables the
"load_components" function (implemented as one function with one env
parameter) to be used in conjunction with searchdirs passed as an
optional parameter:
http://trac.edgewall.org/attachment/ticket/4317/LoaderOptionalSearchdirs.diff
this way, the following external project could use the original
"load_components"
http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58
This would happen without breakage of the existent code.
-
sidenote: Of course, the mega-function "load_components" would need
some more refactoring in order to become more flexible and
maintainable:
http://trac.edgewall.org/browser/trunk/trac/loader.py?rev=rev%3D3557#L34
.
Erik Huelsmann
>
> In order to engourage other projects to use the *original* trac source
> code base instead of writing customized code, a patch acceptance and
> application policy should be provided.
You *should* stop this: filling my mailbox with mails telling others what to do.
bye,
Erik.
Ilias Lazaridis
Jani Tiainen
Ilias Lazaridis wrote:
> Erik Huelsmann wrote:
> > On 12/4/06, Ilias Lazaridis <il...@lazaridis.com> wrote:
> > >
> > > In order to engourage other projects to use the *original* trac source
> > > code base instead of writing customized code, a patch acceptance and
> > > application policy should be provided.
> >
> > You *should* stop this: filling my mailbox with mails telling others what to do.
>
> My threads are valid.
That was rude (not your comment, but Erik's above).
Problem is not that your threads are somehow invalid, you just create
noise and overhead here. You mean well and try to help but meanwhile
aggressively just driving your own needs over core development team. It
just can't work out.
In last month you have posted about 53 messages out of 244, even core
team don't post that much. (closest one is cboos with 34 posts)
So there must be something wrong in the picture?
--
Jani Tiainen
Jani Tiainen
Ilias Lazaridis wrote:
> In order to engourage other projects to use the *original* trac source
> code base instead of writing customized code, a patch acceptance and
> application policy should be provided.
AFAIK it works so that you attach patch to ticket (or create new ticket
if no suitable is found), it's revieved and applied as it fits.
> This is mainly to avoid rejection of patches, e.g. due to personal
> reasons, non-understanding reasons (e.g. due to missing time) etc.!
>
> A patch from a contributor A could be rejected, but the patch would be
> of benefit for the reusability of the trac-code-base, and thus for all
> other users.
>
> A example rule of such policy could be:
>
> * A patch which makes a function general, thus it can be used from
> outside of trac-application
> * should be applied *immediately*, if the existent behaviour is not
> broken.
Who makes decision that patch doesn't break existing behaviour?
> -
>
> An example:
>
> This patch would pass without *any* discussion, as it enables the
> "load_components" function (implemented as one function with one env
> parameter) to be used in conjunction with searchdirs passed as an
> optional parameter:
>
> http://trac.edgewall.org/attachment/ticket/4317/LoaderOptionalSearchdirs.diff
>
> this way, the following external project could use the original
> "load_components"
>
> http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58
>
> This would happen without breakage of the existent code.
Again.. Who has decided it doesn't break existent code? Is it
guaranteed that it doesn't contain bugs?
What if patch contains some malicious stuff? It don't break anything
but it's according to rules stated here to get it applied without any
discussion.
What if patch is not in line with coding style, naming conventions and
such? You suggested that it still would be applied without any
discussion.
Only a trivial patch would be handled this way. Since existence of such
patch is just a hoax, it's pretty much unfeasible.
--
Jani Tiainen
Erik Huelsmann
>
> Ilias Lazaridis wrote:
> > Erik Huelsmann wrote:
> > > On 12/4/06, Ilias Lazaridis <il...@lazaridis.com> wrote:
> > > >
> > > > In order to engourage other projects to use the *original* trac source
> > > > code base instead of writing customized code, a patch acceptance and
> > > > application policy should be provided.
> > >
> > > You *should* stop this: filling my mailbox with mails telling others what to do.
> >
> > My threads are valid.
>
> That was rude (not your comment, but Erik's above).
I'm sorry, but I perceive Ilias's mails as rude too. What was his
actual contribution so far? Who is he to start prescribing others what
they should or should not do? Why can't he just submit actual
proposals and ask for comment/votes on them?
> Problem is not that your threads are somehow invalid, you just create
> noise and overhead here. You mean well and try to help but meanwhile
> aggressively just driving your own needs over core development team. It
> just can't work out.
>
> In last month you have posted about 53 messages out of 244, even core
> team don't post that much. (closest one is cboos with 34 posts)
>
> So there must be something wrong in the picture?
bye,
Erik.
Ilias Lazaridis
> On 12/4/06, Jani Tiainen <red...@gmail.com> wrote:
> > Ilias Lazaridis wrote:
> > > Erik Huelsmann wrote:
http://dev.lazaridis.com/base/wiki/PublicConversation#OffTopic
.
Jeroen Ruigrok/asmodai
>http://dev.lazaridis.com/base/wiki/PublicConversation#OffTopic
Don't come with this holier than thou mentality Ilias, there's ample evidence
of you hijacking other people's threads without staying on topic solely to
draw attention to your things.
--
Jeroen Ruigrok van der Werven <asmodai(-at-)in-nomine.org> / asmodai
イェルーン ラウフロック ヴァン デル ウェルヴェン
http://www.in-nomine.org/
Seize from every moment its unique novelty and do not prepare your joys...
Ilias Lazaridis
> Ilias Lazaridis wrote:
> > In order to engourage other projects to use the *original* trac source
> > code base instead of writing customized code, a patch acceptance and
> > application policy should be provided.
>
> AFAIK it works so that you attach patch to ticket (or create new ticket
> if no suitable is found), it's revieved and applied as it fits.
yes, that's the existent process...
> > This is mainly to avoid rejection of patches, e.g. due to personal
> > reasons, non-understanding reasons (e.g. due to missing time) etc.!
...which leads to unnecessary rejection of patches.
> > A patch from a contributor A could be rejected, but the patch would be
> > of benefit for the reusability of the trac-code-base, and thus for all
> > other users.
> >
> > A example rule of such policy could be:
> >
> > * A patch which makes a function general, thus it can be used from
> > outside of trac-application
> > * should be applied *immediately*, if the existent behaviour is not
> > broken.
>
> Who makes decision that patch doesn't break existing behaviour?
sometimes simple logic, see below.
> > An example:
> >
> > This patch would pass without *any* discussion, as it enables the
> > "load_components" function (implemented as one function with one env
> > parameter) to be used in conjunction with searchdirs passed as an
> > optional parameter:
> >
> > http://trac.edgewall.org/attachment/ticket/4317/LoaderOptionalSearchdirs.diff
> >
> > this way, the following external project could use the original
> > "load_components"
> >
> > http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58
> >
> > This would happen without breakage of the existent code.
>
> Again.. Who has decided it doesn't break existent code? Is it
> guaranteed that it doesn't contain bugs?
there are refactoring steps which can be even automated, thus a machine
could decide that existent behaviour is not broken.
> What if patch contains some malicious stuff? It don't break anything
> but it's according to rules stated here to get it applied without any
> discussion.
Means a requirement must be added (although "should not contain
malicious stuff" should be an obivous requirement).
> What if patch is not in line with coding style, naming conventions and
> such? You suggested that it still would be applied without any
> discussion.
you can add "coding style" and "naming conventions" to the "patch
requirements list".
> Only a trivial patch would be handled this way. Since existence of such
> patch is just a hoax, it's pretty much unfeasible.
And those trivial patches are the 'refactoring patches'.
Many simple, trivial, tiny patches.
Reviewers can verify them against standard-requirements (to give a
'pre-ok').
A few committers, which apply the pre-ok patches after the final
review.
This can bring a code-base step by step into a healthier status.
And this allows contributors to reuse the original code, as they have
the guarantee that "mechanical refactoring patches" get applied anyway
(if they follow naming guidelines etc.).
.
Ilias Lazaridis
> -On [20061204 16:09], Ilias Lazaridis (il...@lazaridis.com) wrote:
> >http://dev.lazaridis.com/base/wiki/PublicConversation#OffTopic
>
> Don't come with this holier than thou mentality Ilias,
My name is Lazaridis.
Mr. Lazaridis.
.
Brad Anderson
> -On [20061204 16:09], Ilias Lazaridis (il...@lazaridis.com) wrote:
>> http://dev.lazaridis.com/base/wiki/PublicConversation#OffTopic
>
> Don't come with this holier than thou mentality Ilias, there's ample evidence
> of you hijacking other people's threads without staying on topic solely to
> draw attention to your things.
>
Jeroen,
I made a mistake. I fed the troll. I know better, and I'm sorry. Let's all
learn from me.
BA
John Hampton
> Jani Tiainen wrote:
>> Ilias Lazaridis wrote:
>>> In order to engourage other projects to use the *original* trac source
>>> code base instead of writing customized code, a patch acceptance and
>>> application policy should be provided.
Well, seems to me that encouraging other projects to use trac code isn't
the first (nor second, likely) priority of Trac and the dev team.
While I don't think they should make decisions lightly that will
purposely hinder other projects from using Trac as a base, I don't
really see this as being a good reason for implementing a "patch
acceptance and application policy"
>>> This is mainly to avoid rejection of patches, e.g. due to personal
>>> reasons, non-understanding reasons (e.g. due to missing time) etc.!
>
> ...which leads to unnecessary rejection of patches.
And who determines what is an unnecessary rejection? This all boils
down to differences in opinions and priorities. And when you get to
that level, the devs have ultimate authority. So if it's necessary to
them, then it's basically necessary to all.
>>> A patch from a contributor A could be rejected, but the patch would be
>>> of benefit for the reusability of the trac-code-base, and thus for all
>>> other users.
As I stated above, while making it easy for other projects to use as a
base is nice, it's not a priority. Also, it's hard to decide what
consequences a simple patch might have in the future. Sure, you provide
a nice interface now, and lots of external projects start using it, but
what happens when you realize that to further Trac, you need to break
that interface which really only serves external projects? Sure, you
can go ahead and simply break it in the name of progress, but then you
have tons of external projects complaining.
>>> A example rule of such policy could be:
>>>
>>> * A patch which makes a function general, thus it can be used from
>>> outside of trac-application
>>> * should be applied *immediately*, if the existent behaviour is not
>>> broken.
A policy like this introduces too many patches for too little gain. See
previous paragraph re: unintended future consequences.
>>> This patch would pass without *any* discussion, as it enables the
>>> "load_components" function (implemented as one function with one env
>>> parameter) to be used in conjunction with searchdirs passed as an
>>> optional parameter:
While I'm actually for the stated patch itself, having any patch be
applied "without *any* discussion" due only to the fact that "existent
behaviour is not broken" is a VERY bad idea, IMNSHO. You're opening
yourself up to abuse and lots more complaints. The fact remains that
the devs have absolute authority over which patches they accept and
which they don't. The should not be holden to some stupid policy
telling them which patches they MUST accept. If the time comes that all
the Trac devs have suffered severe brain damage and are hindering the
project because of their poor decisions and refusal to accept patches,
then the project will be forked[1]. That's the OSS way ;)
> This can bring a code-base step by step into a healthier status.
Who determines the health of the code base? For what I've read of the
trac code, which is a fair amount, it is generally of very high quality.
> And this allows contributors to reuse the original code, as they have
> the guarantee that "mechanical refactoring patches" get applied anyway
> (if they follow naming guidelines etc.).
Guarantees of that sort are bad. OSS is generally seen as a
meritocracy. Which means that the best code is accepted. Having some
policy (which is hard enough to define) that is used as a rule by which
code is accepted, is a BAD idea. I have said it before, and I'll say it
again. The Trac devs have the ultimate authority on what they accept
and what they don't. Any attempt to change this is bad. I trust their
decisions, direction, and competence. And to their credit, look where
it has brought trac.
-John
[1] However, from getting to know the Trac devs, I think we are VERY,
VERY far from this.
datenimperator
I'm silently participating in this group, and during the last few weeks
I was wondering why there was a new name that occurred to me much more
often than the rest of the names of the team that I happen to know. Is
it just me (and, perhaps, the GoogleGroups "profile" function)
believing that somebody has a habit of trying to "take over" opensource
development?
As far as I'm concerned, the trac team does a good job (if you can
speak of a "job" regarding some free, donated piece of work) designing
and implementing something that's awfully useful for my daily business.
While I'm pretty aware that following something known as "best
practice" and "defined process" will help projects to eventually
succeed, I happen to work a lot with people that care more about the
"how" than the "what". Pesonally, I tend to dislike that, and I'm happy
to see a more pragmatic approach here.
And I'd like it to stay that way. Ilias (may I call you by your first
name?), you have this web page about a trac audit.
http://case.la[xxx].com/wiki/TracAudit
May I ask who asked you to do a audit at all, why this audit or its
outcomings should influence the trac development and why it's you
setting up rules that others should follow? Kind regards,
Christian
Noah Kantrowitz
If either a) do not trust the development team (see earlier thread
about editing tickets), or b) disagree with their priorities then
simply do not use the piece of software. This is not specific to
either Trac or Ilias. A fork has been suggested as an option, and
there are other tools that could used. I am personally a big fan of
forks in FOSS, I think they allow more people to bring their ideas to
the table. For those that don't know, DrProject is a fork of Trac
from a while ago, and I think it has some interesting features that
would be nice in Trac. This is the way projects feed each other. Can
we turn this into a constructive debate? Clearly you, Ilias, dislike
a large number of Trac's policies or lack thereof, so take the source
and go work on it yourself. If you come up with good ideas, I am sure
someone will notice. If you make something better than Trac, I am
sure people will start using it. As I said at the beginning, if
either of those two statements apply to you, you need to do something
differently, not the Trac team.
--Noah Kantrowitz
Plugin Guru
Sebastien Douche
> I'm sorry, but I perceive Ilias's mails as rude too. What was his
> actual contribution so far? Who is he to start prescribing others what
> they should or should not do? Why can't he just submit actual
> proposals and ask for comment/votes on them?
Don't feed the troll :)
For more information on him : http://en.wikipedia.org/wiki/Ilias_Lazaridis
--
Sébastien Douche <sdo...@gmail.com>
Jani Tiainen
Ilias Lazaridis wrote:
> Jani Tiainen wrote:
> > Ilias Lazaridis wrote:
> > > In order to engourage other projects to use the *original* trac source
> > > code base instead of writing customized code, a patch acceptance and
> > > application policy should be provided.
> >
> > AFAIK it works so that you attach patch to ticket (or create new ticket
> > if no suitable is found), it's revieved and applied as it fits.
>
> yes, that's the existent process...
And works pretty well from personal experience.
> > > This is mainly to avoid rejection of patches, e.g. due to personal
> > > reasons, non-understanding reasons (e.g. due to missing time) etc.!
>
> ...which leads to unnecessary rejection of patches.
Depends who decides what is "unnecessary" and which is not.
> > > A patch from a contributor A could be rejected, but the patch would be
> > > of benefit for the reusability of the trac-code-base, and thus for all
> > > other users.
> > >
> > > A example rule of such policy could be:
> > >
> > > * A patch which makes a function general, thus it can be used from
> > > outside of trac-application
> > > * should be applied *immediately*, if the existent behaviour is not
> > > broken.
> >
> > Who makes decision that patch doesn't break existing behaviour?
>
> sometimes simple logic, see below.
Meaning by blindly trusting? Oh, set of rules involving human
intervention... (note the sarcasm here)
> > > An example:
> > >
> > > This patch would pass without *any* discussion, as it enables the
> > > "load_components" function (implemented as one function with one env
> > > parameter) to be used in conjunction with searchdirs passed as an
> > > optional parameter:
> > >
> > > http://trac.edgewall.org/attachment/ticket/4317/LoaderOptionalSearchdirs.diff
> > >
> > > this way, the following external project could use the original
> > > "load_components"
> > >
> > > http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58
> > >
> > > This would happen without breakage of the existent code.
> >
> > Again.. Who has decided it doesn't break existent code? Is it
> > guaranteed that it doesn't contain bugs?
>
> there are refactoring steps which can be even automated, thus a machine
> could decide that existent behaviour is not broken.
Machine could, but it can't. It can only run predefined set of tests
and see if output is same. It can't never be as intuitive as enduser.
> > What if patch contains some malicious stuff? It don't break anything
> > but it's according to rules stated here to get it applied without any
> > discussion.
>
> Means a requirement must be added (although "should not contain
> malicious stuff" should be an obivous requirement).
But then it would need attention, reviewing?
> > What if patch is not in line with coding style, naming conventions and
> > such? You suggested that it still would be applied without any
> > discussion.
>
> you can add "coding style" and "naming conventions" to the "patch
> requirements list".
More need for manpower to review patch...
> > Only a trivial patch would be handled this way. Since existence of such
> > patch is just a hoax, it's pretty much unfeasible.
>
> And those trivial patches are the 'refactoring patches'.
>
> Many simple, trivial, tiny patches.
There is no such thing as trivial patch.
> Reviewers can verify them against standard-requirements (to give a
> 'pre-ok').
>
> A few committers, which apply the pre-ok patches after the final
> review.
Um... Isn't that contradictory with original idea to get it commited
without discussion, immediately..?
Now you're saying that it must be reviewed (and thus maybe discussed,
or even rejected).
You can't have both immediate applying "trivial" patches and reviews to
some ruleset, unless you get someone to watch patches 24/7...
> This can bring a code-base step by step into a healthier status.
Determined by who and by which factors?
> And this allows contributors to reuse the original code, as they have
> the guarantee that "mechanical refactoring patches" get applied anyway
> (if they follow naming guidelines etc.).
In collaborative environment you can't really do refactoring (except
method extracting) without breaking "original code".
--
Jani Tiainen
Ilias Lazaridis
monitor/respond anymore).
-
summary for readers:
this patch:
http://trac.edgewall.org/attachment/ticket/4317/LoaderOptionalSearchdirs.diff
should introduce an initial refactoring to this code:
http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58
in order to enable reuse in different usage-contexts, e.g.:
http://trac.edgewall.org/browser/trunk/trac/loader.py?rev=rev%3D3557#L34
-
It was suggested to provide a patch-acceptance-policy in order to
ensure that simple refactoring patches are accepted with an
semi-automated process.
.