buildr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ittay Dror <ittay.d...@gmail.com>
Subject Re: [PATCH] added -p switch to specify project name
Date Mon, 01 Sep 2008 03:27:55 GMT


Assaf Arkin wrote:
> 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
>   
I realized after posting that my language was blunt. Sorry about that 
and thank you for being level headed.

Ittay
>   
>>> 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>
>>
>>
>>     

-- 
--
Ittay Dror <ittay.dror@gmail.com>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message