Top posting. Despite my current skepticism, apparently in the (embarrassingly recent :-)) past I found svn update enough of a common pattern to factor out updates:
https://github.com/apache/whimsy/blob/master/www/board/agenda/views/actions/publish.json.rb The implementation of this method does use a block with yield: https://github.com/apache/whimsy/blob/master/lib/whimsy/asf/svn.rb I've now added a method to go a combination of svn info and cat; the need to log or intercede in those methods isn't quite as strong, so that is more easily factored out. The implementation of ASF::SVN.update could be changed to make use of ASF::SVN.get and svnmucc, and eliminate the need for temporary directories. - Sam Ruby On Thu, Mar 31, 2016 at 9:59 AM, sebb <seb...@gmail.com> wrote: > On 31 March 2016 at 12:20, Sam Ruby <ru...@intertwingly.net> wrote: >> On Thu, Mar 31, 2016 at 6:25 AM, sebb <seb...@gmail.com> wrote: >>> On 31 March 2016 at 02:19, Sam Ruby <ru...@intertwingly.net> wrote: >>>> On Wed, Mar 30, 2016 at 6:06 PM, sebb <seb...@gmail.com> wrote: >>>>> I just realised that we could use the svnmucc 'put' command to >>>>> simplify adding/modifying a file. >>>>> >>>>> There is no need to create a temporary checkout of the parent directory. >>>>> >>>>> One can even use '-' to put from standard input. >>>> >>>> Thanks! Much cleaner: >>>> >>>> https://github.com/apache/whimsy/commit/8e74fe0267058e919b85380c956295e76689c2d9 >>> >>> The -r 0 qualifier is neat. I'd not noticed that before. >> >> If you don't pass that, svnmucc will overwrite existing content. >> >>> Not sure why you use stdin since the file already exists? >>> Why not pass the path on the command line? >> >> The file doesn't exist in this case - it is an html form parameter. >> @file is an object that responds to .read. > > I see, I assumed it was the file path name. > >> Here's the relevant >> portion of wunderbar's implementation of _.system: >> >> https://github.com/rubys/wunderbar/blob/master/lib/wunderbar/builder.rb#L94 >> >>>>> Won't be suitable for everything, but might prove useful. >>>>> >>>>> Maybe worth creating a function to implement this? >>>> >>>> I remain unclear as to the value of a function, but you are welcome to >>>> explore that. >>>> >>>> Note that _.system, when called within _html, will create <pre >>>> class="_stdin'> and <pre class="_stdout"> elements that are placed in >>>> the html output. >>> >>> Yes, I saw. >>> >>> How does one call it outside _html? >> >> Well, you wouldn't generally call that _.system method outside of >> _html, but I have similar methods for _json and _text and for >> websockets that format and include the results in the output. > > Would it not also be useful for inline ruby code where the output > needs to be processed further by the script? > >>>> The board agenda tool has retry logic. I don't yet see the common >>>> logic that merits refactoring, but perhaps you see something I have >>>> missed? >>>> >>>> And if truly common code, I'm open to including it in wunderbar. >>> >>> I suppose the required code as above is quite short. >>> >>> However it might be useful to create a version (or verions) that handle: >>> - put if absent (-r 0) >>> - put unconditionally >>> - put if newer (-r n) >>> >>> Also the function could add the standard ['--no-auth-cache', >>> '--non-interactive'] qualifiers. >> >> What generally is even more useful in Ruby is to do this as a block. >> In the board agenda tool updating the agenda is a common function, so >> that posting a list of action items calls Agenda.update passing a >> block. For example: >> >> https://github.com/apache/whimsy/blob/master/www/board/agenda/views/actions/post-actions.json.rb >> >> The call to Agenda.update passes a block (do...end), and that block is >> called (via a yield) with the current agenda contents and is expected >> to return the updated contents. > > I see. > > That's a design method I've not had much experience with, so I don't > tend to think of it when writing code. > Must give it more attention. > >> The definition of Agenda.update is handles not only svn, but file >> locks, capturing output, updating the cache, and retries: >> >> https://github.com/apache/whimsy/blob/master/www/board/agenda/models/agenda.rb#L82 >> >> Updating the cache is what broadcasts updates to other users of the >> agenda tool that a change has happened, so everybody's client will see >> the new content pretty much instantly. Wunderbar's implementation of >> _.system spins off three threads, one each to deal with stdin, stdout, >> and stderr. > > Yes, that is what I think would be potentially useful from inline code > >> Looking at how treasurer/bill_upload and board/agenda do svn updates, >> I don't see much in common that would benefit from refactoring out. >> But within the board agenda tool, there clearly was a common pattern >> that was refactored into a common method for that one tool. And I do >> see how board/agenda could benefit from svnmucc. But if you see >> something that is in common that could benefit from refactoring out, >> go for it... > > I was thinking also that should there later be a better way of > invoking svn commands - e.g. a direct library call rather than > involving the shell - it would be a lot easier to implement if the > svn/svnmucc commands were isolated in a single method. > >> In general, the way whimsy tools have been developed to date is to >> focus first on writing the tool, and then to look for common code >> patterns that could benefit from being moved to a common library. >> wunderbar predated whimsy/asf, and started out as code that I commonly >> added when I made use of the builder gem. In fact there still is one >> tool (collate_minutes that produces board/minutes) that hasn't been >> converted from builder to wunderbar. >> >>> It looks as though svnmucc does not honour config [auto-props] so that >>> could be added to the put if absent code. >>> >>>> - Sam Ruby >> >> - Sam Ruby
