buildr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Assaf Arkin" <ar...@intalio.com>
Subject Re: [PATCH] added -p switch to specify project name
Date Mon, 01 Sep 2008 00:13:57 GMT
On Sun, Aug 31, 2008 at 1:52 PM, Ittay Dror <ittay.dror@gmail.com> wrote:
>
>
> Assaf Arkin wrote:
>>
>> On Sun, Aug 31, 2008 at 1:02 AM, Ittay Dror <ittayd@tikalk.com> wrote:
>>
>>>
>>> This patch adds the ability to run buildr as 'buildr -p <project name>',
>>> instead of 'cd' to the project's base directory. This is more comfortable
>>> when building several projects and when using buildr as a tool from an
>>> ide
>>> (since specifying an argument is easier than specifying a working dir)
>>>
>>
>> A few suggestions:
>>
>> 1.  Follow procedures.
>>
>> http://incubator.apache.org/buildr/contributing.html#contributing_code
>>
>> 2.  Describe the problem, then offer a solution.
>>
>> Start with why before getting to how. If the patch itself looks like
>> it's wrong in more ways than one, it will get a WONTFIX.
>>
>> If you start by describing the problem, you can get +1 from other
>> people who feel the same pain, and you allow us to propose and
>> consider different solutions.
>>
>
> from past experience working with projects a subject is kept more lively
> when discussed in a mailing list, not through comments in the issue tracker
> (even if the issue tracker uses mail notifications).
>>
>> 3.  Do the necessary leg work.
>>
>> Explain what else you tried that didn't work, what could be affected
>> by the patch, if it's inconsistent with other design decisions, why
>> you chose to do it that way.
>>
>> If from a quick read it looks like none of that was considered, the
>> patch won't be considered as well. JIRA has infinite space for patches
>> to sit there collecting dust.
>>
>
> i think this is sad. buildr is at an early stage where i think you should
> really give people a chance, not put barriers in front of them. i'm
> following the git mailing list and see lots of people say stupid things or
> submit incomplete patches. they are always dealt with respect, giving them
> comments and allowing them to resubmit (junio lately appologized that he now
> no longer has the time to privately nag people if they don't resubmit a
> patch after being reviewed).

We have a constraint of too many open issues and killer ideas and not
enough people to resolve them all.  That means even the best idea
could be left out to dry or get rejected because we don't have
bandwidth to deal with it.  It might be that we just don't know what
it solves, or what the side effects are, or how it fits with relation
to other things.  We have easy patches that gets applied the very same
day, and we have hard patches that languish in the queue until we get
around to deal with them, if ever.  I don't like this constraint, but
until we build a bigger community here, we'll have to learn to live
with it.

The best I can do right now is come up and say "here are the red
flags" and then we can turn around and see how to deal with those
flags.  If we can turn this into an easy issue, we'll get it solved
much faster.  Obviously you're very passionate about it, so the wrong
thing would be to reject it on account of these red flags.  Rather we
should discuss what problem we're trying to solve and from that how to
best solve it.

Assaf

>>
>> 4.  Hold on until you're ready.
>>
>> We have to look at the patch, understand it, review it, decide if we
>> want to incorporate it, if it needs any fixing. If it keeps changing
>> every time I look at it, I'll just stop looking at it.
>>
>> But mostly consider that there's a lot of effort required to accept
>> even the simplest one-line change. A change in one place could break
>> something else. It may be inconsistent with other design decisions, we
>> don't want a hodgepodge of patches. It may fix a symptom but not the
>> actual problem., or conflict with another proposed change. Then we
>> have to review, test, document and maintain it.
>>
>
> I think a simple review of the patch can reveal if it may break other
> features and running the rspecs increases confidence. Again I like the git
> mailing list here.
>>
>> Specifically for this patch:
>> - You can already build a project without changing into its directory.
>> http://incubator.apache.org/buildr/projects.html#running_project_tasks
>>
>> - Command line options are used for controlling Buildr.application,
>> variables for controlling tasks.
>>
>> - Follow coding conventions.
>>
>> - return projects if projects: when is projects ever nil?
>>
>> - local_projects expects a block.
>>
>> Assaf
>>
>>
>>
>>>
>>> ---
>>> lib/buildr/core/application_cli.rb |    6 +++++-
>>> lib/buildr/core/project.rb         |    4 ++++
>>> 2 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/lib/buildr/core/application_cli.rb
>>> b/lib/buildr/core/application_cli.rb
>>> index 3a19cf9..3f826e8 100644
>>> --- a/lib/buildr/core/application_cli.rb
>>> +++ b/lib/buildr/core/application_cli.rb
>>> @@ -59,7 +59,9 @@ module Buildr
>>>       ['--version',  '-v', GetoptLong::NO_ARGUMENT,
>>>         'Display the program version.'],
>>>       ['--environment', '-e', GetoptLong::REQUIRED_ARGUMENT,
>>> -          'Environment name (e.g. development, test, production).']
>>> +          'Environment name (e.g. development, test, production).'],
>>> +        ['--project',  '-p', GetoptLong::REQUIRED_ARGUMENT,
>>> +          'Project name, can be relative to current directory']
>>>     ]
>>>
>>>   def collect_tasks
>>> @@ -99,6 +101,8 @@ module Buildr
>>>       options.show_task_pattern = Regexp.new(value || '.')
>>>     when '--nosearch', '--quiet', '--trace'
>>>       super
>>> +      when '--project'
>>> +         options.project = value
>>>     end
>>>   end
>>>
>>> diff --git a/lib/buildr/core/project.rb b/lib/buildr/core/project.rb
>>> index 6a37751..d5c511a 100644
>>> --- a/lib/buildr/core/project.rb
>>> +++ b/lib/buildr/core/project.rb
>>> @@ -336,6 +336,10 @@ module Buildr
>>>     end
>>>
>>>     def local_projects(dir = nil, &block) #:nodoc:
>>> +        if dir.nil? and Buildr.application.options.project
>>> +          projects = local_projects('.').map{|p|
>>> project("#{p}:#{Buildr.application.options.project}")}
>>> +          return projects if projects
>>> +        end
>>>       dir = File.expand_path(dir || Buildr.application.original_dir)
>>>       projects = Project.projects.select { |project| project.base_dir ==
>>> dir }
>>>       if projects.empty? && dir != Dir.pwd && File.dirname(dir)
!= dir
>>> --
>>> 1.6.0.36.g3814c
>>>
>>> --
>>> Ittay Dror <ittayd@tikalk.com>
>>> Tikal <http://www.tikalk.com>
>>> Tikal Project <http://tikal.sourceforge.net>
>>>
>>>
>>>
>>>
>
> --
> --
> Ittay Dror <ittay.dror@gmail.com>
>
>

Mime
View raw message