[darcs-users] darcs patch: modify issue1300 test to fail for the ri... (and 2 more)

Kamil Dworakowski kamil at dworakowski.name
Thu Sep 17 09:02:32 UTC 2009


Hi, Eric

On Thu, Sep 17, 2009 at 8:56 AM, Eric Kow <kowey at darcs.net> wrote:
>>  Don't honour --delete-logfile when the record fails for any reason, a test
>>  failure for instance.
>
>>  I have changed the definition of get_log not to delete the logfile, but to
>>  return it for deferred deletion. I capitalized on the fact that get_log
>>  was already returning (Just temp_logfile) for deferred deletion.
>>
>>  get_log is an exported name, used in AmendRecord, Tag and Rollback. Some
>>  of them ignore the logfile to delete, though none of them accept
>>  --delete-logfile flag, and thus they don't need to change, nor are they
>>  affected in any way.
>>
>>  Unintended side effect of the change: with --delete-logfile flag present, a
>>  massage '"Logfile left in " ++ filepath' gets printed in case of the test
>>  failure on record. This may actually be desirable so I did not bother to
>>  change it.
>
> This is a fine log, but it should be made more concise :-)
> --edit-description is a good place for some stuff.
>
> I don't have any fixed rules about this by the way (yet); it's just a
> sort of "feeling" for what will be most useful when you're trying to
> study the darcs changes output in the future.

This is interesting for me. If I were studying changes I would like to have
all the relevant information in the patch log rather then have incomplete
information there and possibly some relevant information on the mailing list.

Is this only about some fuzzy line between what information should go
into the patch log and what should go into the post description? Obviously
greeting and such belong only to the post. My justification of the amendment
also belongs to the post not to the log. Though in retrospect I could have
put the justification for replacing removeFileMayNotExist with unchecked
deferring of deletion (later it is removed using removeFile) inside the log.

> -                 when (RmLogFile `elem` opts) $ removeFileMayNotExist f
> -                 return (name, thelog, Nothing)
> +                 let toRemove = if RmLogFile `elem` opts
> +                        then Just $ toFilePath f
> +                        else Nothing
> +                 return (name, thelog, toRemove)

I am not at all attached to this view. It was just an idea I had when I was
composing this bundle. I though that if I will explain the changes for the
list/reviewer anyway, why not include it in the commit log for anyone who
will later be studying the history of the repository.

Just to ensure we are on the same page. I interpreted you saying that the
"log should be more concise", as "it contains to much information", because
it seems concise to me for the information it contains.

--
Cheers,
Kamil


More information about the darcs-users mailing list