Hi kou
On 8/10/07, Kouhei Sutou <kou@cozmixng.org> wrote:
> Hi,
>
> > changed the apis. I just did a diff between the 1.4.x branch and
> > trunk and I think update_editor has a new required argument (perhaps
> > just a rename, though?).
>
> Ah, it's not good... Sorry.
> We should define update_editor2 for Wc.get_update_editor3
> and keep update_editor API. Could you fix my faults?
I'll look at it later. See below.
>
> > > Of cause we should validate arguments of hash argument
> > > version methods (xxx_yyy2). We'll need to use the following
> > > method:
> > >
> > > class Hash
> > > def assert_valid_keys(*valid_keys)
> > > unknown_keys = keys - [valid_keys].flatten
> > > unless unknown_keys.empty?
> > > raise(ArgumentError, "Unknown key(s): #{unknown_keys.join(", ")}")
> > > end
> > > end
> > > end
> > >
> > > # The above method is quoted from ActiveSupport.
> > >
> >
> > I'm not familiar with this. Is it just to ensure that there were no
> > typos in the passed in options?
>
> Yes.
>
> > I was thinking it might be nice just
> > to ignore unrecognized options. I haven't given it a lot of thought,
> > but it seemed like clients might be able to create their own options
> > hashes, and their might be room for reuse if we ignored unrecognized
> > keys.
>
> In my experience, there aren't many cases to reuse option
> hash but there are many typos in scripts. :<
> I think a case that we trust our script's logic doesn't have
> any problems but the script doesn't work is too costly (we
> will use much time and be damaged our mind). And most of
> causes of the case are caused by a typo!
>
> So I want to validate hash options. We can remove validation
> after many people request.
>
That is reasonable. Obviously, I hadn't tried making such a hash yet
so I will accept your judgment that it won't help. Anyway, being so
explicit will probably make it easier to document and understand which
options are valid for each function.
What I am planning to do:
1) Finish getting the tests to pass. The client test_merge and
test_merge_peg are still broken (I think there have been at least
three revisions that have contributed to their brokenness). I have
them passing locally, but I'm going to spend some time trolling the
history to see if I can figure out exactly what revisions did the
breaking.
2) Change all the merge_info to mergeinfo and get rid of the aliases.
3) Revert update_editor and switch_editor changes and add
update_editor2 and switch_editor2 with the hash options as we discuss
above.
4) More diffs with 1.4.x to see if I can spot any other api regressions.
5) Start adding more options hashes (should we come up with a list of
methods with many optional arguments to prioritize?).
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 11 16:51:30 2007