Re: [Monotone-devel] options for automate inventory

[Thomas Keller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stephen Leake schrieb:
>> 2) From what I can see in the code you determine if a file stanza should
>> be omitted from the inventory_print_states and inventory_print_changes
>> methods. 
> 
> Right. But since each only knows part of the information, they can't
> just output 'print_this' directly.

Yeah, this ugly hick-a-hack I've done there. I'm not satisfied with it.
Well - your decision.

>> If you really go that route, the three boolean values should at
>> least be set to false _outside_ of the first method (currently
>> they're initialized in the body), 
> 
> Ah. That's a difference between Ada (my everyday language) and C++; in
> Ada, you can declare a subprogram parameter to be "in", "in out" or
> "out", so it is clear whether you can rely on the initial value. In
> C++, it is not clear whether a "&" parameter is "in out" or just
> "out"; I was intending these to be "out" for inventory_print_states,
> since it doesn't use the initial value, and "in out" for
> inventory_print_changes, since it does.

I think subprog parameters can only be in and in-out in c/c++, but
others may correct me here. The reason why I'd define them outside is
just to not give anyone false assumptions when he/she initializes the
variables with another value than "false" before the function call,
because this initial value gets always overwritten inside unconditionally.

>> but I really think the actual filtering should be done beforehand
>> and not be mixed into these two methods. I know this involves a
>> litte code duplication, but its far easier to understand later on.
> 
> But not to maintain; it would be too easy to change one routine
> without changing the other.
> 
> If these procedure names where changed to "inventory_compute_state",
> "inventory_compute_changes", would that make it clearer?
> 
> Do the comments I've added help?

Which rev? Have you pushed it already?

>> 4) A minor thing: the interface_version does not neccessarily need to be
>> bumped to 7.1, since mtn 0.38 is 6.0 anyways... and I *think* there was
>> a previously used rule that only command additions get a minor raise,
>> however format / output changes of existing commands get a major raise.
>> So either stick with 7.0 (which incorporates the attributes stuff) or
>> raise it to 8.0.
> 
> The comment on interface_version in cmd_automate.cc says:
> 
> // Purpose: Prints version of automation interface.  Major number increments
> //   whenever a backwards incompatible change is made; minor number increments
> //   whenever any change is made (but is reset when major number increments).
> 
> This doesn't say anything about "only one increment per mtn version";
> perhaps it should?
> 
> This counts as "any change", but it is backwards compatible - if you
> don't use the options, you don't notice they got added.
> 
> Hmm. "don't recurse into ignored directories" is not backwards
> compatible. But I see that as bug fix; no one should have been relying
> on that behavior in the first place.

Well, I plan to do some more incompatible changes / fixes to it anyways,
so at least one major increment is needed for 0.39. The thing is that
the interface_version wasn't designed to support "intermediate" versions
of monotone at all. We could define that the version numbering works
that way, but I'm not a fan of this, because external tool developers
might get slightly confused with that ("huh, why did the interface
version jump two digits?") and we have also no defined way to easily
query which interface version was set from what revision - maybe there
should be a special tag for this purpose?

> And I need the interface_version change in my current Emacs DVC code,
> since the monotone version has _not_ changed yet; I need to know
> whether I can use --no-ignored or not. I suppose that's a minor point;
> if I'm using the head of the development branch, I should know what
> I'm doing :).
> 
> Still, it makes more sense to decide whether to use --no-ignored based
> on automate interface_version rather than mtn version.

Thats what interface_version is for - the mtn version may change quite a
few times but the interface could stay constant. So f.e. if 0.40 still
has 7.0 you don't have to code-in support for this new version.

>> It may be ok to do that (I haven't compiled anything from the new
>> texi file, so I can't say for sure everything is ok), 
> 
> I did compile it; no errors. 

Fine!

>> but it would be great if that would happen in a separate patch /
>> revision ;)
> 
> I'm not clear; should I back out this change and just do the doc
> update without the reformat? 
> 
> Or leave it as is?

Well, if others are ok with it, leave it as is (I don't have to decide
that, really ;). Just note that, next time you do wanted/accidential
formatting changes, do that in a separate revision to make it easier for
others to read the diff afterwards.

Thomas.

- --
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFHjIWVaf7NlBYNEJIRAnNmAJ46S5KbPIAPdjtLr2sZnlGCEjo6lwCg0jib
rws3Evt+/EIkWqxNLltVTfQ=
=UkKr
-----END PGP SIGNATURE-----