The make commit function will add this card automatically. It took me a bit
to figure out where it was comming from. I do an hg diff, everything looks
good, then commit and there is this change to a CARDLIST file in the diffstat.
Took me a minute to figure out what was going on. Thought Mercurial was
busted for a bit, but no, it's just darcs that messes up.
The way make commit makes changes without telling you or giving you chance to
review them bothers me. Since not everyone uses make commit, sometimes the
auto-fixes it finds are caused by someone else's patch. You could end up
commiting something and creating a patch with a change you didn't know about
and that has nothing to do with your patch.
I think it would be nicer, if the checks make commit does find a problem, it
doesn't auto-apply them but instead makes a patch file and then tells you,
"there is something wrong with what you are trying to commit, you should make
the changes in this patch file." Then you can see what's going on and apply
the patch if it make sense to.

Trent Piepho wrote:
The make commit function will add this card automatically. It took me a bit
to figure out where it was comming from. I do an hg diff, everything looks
good, then commit and there is this change to a CARDLIST file in the diffstat.
Took me a minute to figure out what was going on. Thought Mercurial was
busted for a bit, but no, it's just darcs that messes up.

The way make commit makes changes without telling you or giving you chance to
review them bothers me. Since not everyone uses make commit, sometimes the
auto-fixes it finds are caused by someone else's patch. You could end up
commiting something and creating a patch with a change you didn't know about
and that has nothing to do with your patch.

I think it would be nicer, if the checks make commit does find a problem, it
doesn't auto-apply them but instead makes a patch file and then tells you,
"there is something wrong with what you are trying to commit, you should make
the changes in this patch file." Then you can see what's going on and apply
the patch if it make sense to.

Mauro,

Please do NT apply this patch from Trent.

There is nothing wrong with this patch, it is 100% correct, and Trent is
right about everything that he says above, however, I have already
taken care of this in my pending hg-pull-request, located at:

http://linuxtv.org/hg/~mkrufky/v4l-dvb

Please pull, to receive the following:
- lgdt330x: fix missing line in VSB snr decoding logic
- update cardlist documentation

| 1 +
| 1 +
2 files changed, 2 insertions(+)

It appears that when you apply patches from the list, Mauro, that the
cardlist generator script does not run before you apply them. I pointed
this problem to you recently with regards to the whitespace cleanup
script, and I believe that is resolved now, but it looks like there is
more work for you to do on your mailimport script.

Regards,

Mike

Michael Krufky wrote:
Trent Piepho wrote:
>The make commit function will add this card automatically. It took
>me a bit
>to figure out where it was comming from. I do an hg diff, everything
>looks
>good, then commit and there is this change to a CARDLIST file in the
>diffstat.
>Took me a minute to figure out what was going on. Thought Mercurial was
>busted for a bit, but no, it's just darcs that messes up.
>>
Actually, this was because of the way that Mauro had applied a previous
patch without using the standard commit procedure. i explained the
problem to Mauro, and I am sure that he can fix this such that it is
prevented in the future.
>The way make commit makes changes without telling you or giving you
>chance to
>review them bothers me. Since not everyone uses make commit,
>sometimes the
>auto-fixes it finds are caused by someone else's patch. You could
>end up
>commiting something and creating a patch with a change you didn't
>know about
>and that has nothing to do with your patch.
>>
This really should never happen The point is for the cardlist
generator and whitespace cleanup scripts to run, cleaning up the patch
as it is committed as stated above, this only happened to you due
to the way that Mauro pushed up a previous commit.
>I think it would be nicer, if the checks make commit does find a
>problem, it
>doesn't auto-apply them but instead makes a patch file and then tells
>you,
>"there is something wrong with what you are trying to commit, you
>should make
>the changes in this patch file." Then you can see what's going on and
>apply
>the patch if it make sense to.

Perhaps, but I actually do like the way it works right now, provided
that Mauro fixes his own patch import scripts.

It is better for these changesets to happen automatically, so that we
dont need to be bothered specifically by whitespace cleanup patches or
CARDLIST documentation additions.

Cheers,

Michael Krufky

Tue, 20 Jun 2006, Michael Krufky wrote:
Michael Krufky wrote:
>end up
>commiting something and creating a patch with a change you didn't
>know about
>and that has nothing to do with your patch.
>>
This really should never happen The point is for the cardlist
generator and whitespace cleanup scripts to run, cleaning up the patch

Shouldn't happen, but, it does. How about if the commit check scripts find
something, they at least tell you? For example, print a warning message you
have time to read before starting the commit. put some kind of message as
a comment in the initial commit message. Like this:

#Added/removed/changed files:
# v4l/Makefile | 3
# CARDLIST.saa7134 | 1 +
# 2 files changed, 3 insertions(+), 1 deletion(-)
#
# Note, a problem with your patch was detected! These changes were made
# automatically for you:
# CARDLIST.saa7134 | 1 +
# 1 files changed, 1 insertion(+)
#
# Please review these changes and see if they belong in your patch.
#
# For better log display, please keep a blank line after subject, after from
# and before signed-off-by
# [and so on]

dont need to be bothered specifically by whitespace cleanup patches or

BTW, there is a huge number of whitespace errors in files outside the linux
directory, which weren't auto-checked. If they are not fixed all at once,
they will be auto-fixed as people commit patches to those files.

Trent Piepho wrote:
Tue, 20 Jun 2006, Michael Krufky wrote:

>>Michael Krufky wrote:
>>
end up
commiting something and creating a patch with a change you didn't
know about
and that has nothing to do with your patch.

>>
>>This really should never happen The point is for the cardlist
>>generator and whitespace cleanup scripts to run, cleaning up the patch

Shouldn't happen, but, it does. How about if the commit check scripts find
something, they at least tell you? For example, print a warning message you
have time to read before starting the commit. put some kind of message as
a comment in the initial commit message. Like this:

#Added/removed/changed files:
# v4l/Makefile | 3
# CARDLIST.saa7134 | 1 +
# 2 files changed, 3 insertions(+), 1 deletion(-)
#
# Note, a problem with your patch was detected! These changes were made
# automatically for you:
# CARDLIST.saa7134 | 1 +
# 1 files changed, 1 insertion(+)
#
# Please review these changes and see if they belong in your patch.
#
# For better log display, please keep a blank line after subject, after from
# and before signed-off-by
# [and so on]

Something like this would be fine with me. In fact, I think that it is
a great idea!

>>dont need to be bothered specifically by whitespace cleanup patches or

BTW, there is a huge number of whitespace errors in files outside the linux
directory, which weren't auto-checked. If they are not fixed all at once,
they will be auto-fixed as people commit patches to those files.

When we had originally set up the automatic whitespace cleanup scripts,
they were set to only check the linux directory. This is the only
place where we keep kernel related files, and thus the only files that
need whitespace cleaning. TH, it would be nice for the test/
directory to be cleaned as well, but this isnt so important. (at least
not yet)

Regardless, the script does not necessarily check files that have been
patched. The script *only* checks the linux directory for whitespace
problems.
-Mike

Tue, 20 Jun 2006, Michael Krufky wrote:
When we had originally set up the automatic whitespace cleanup scripts,
they were set to only check the linux directory. This is the only
place where we keep kernel related files, and thus the only files that
need whitespace cleaning. TH, it would be nice for the test/
directory to be cleaned as well, but this isnt so important. (at least
not yet)

Regardless, the script does not necessarily check files that have been
patched. The script *only* checks the linux directory for whitespace
problems.

It did only check linux. I wrote a new faster one, that checks the files being
comitted. I could modify it to only check the linux directory, but why not
check the scripts and everything else?

Tue, 20 Jun 2006, Michael Krufky wrote:
Trent Piepho wrote:
have time to read before starting the commit. put some kind of message as
a comment in the initial commit message. Like this:

#Added/removed/changed files:
# v4l/Makefile | 3
# CARDLIST.saa7134 | 1 +
# 2 files changed, 3 insertions(+), 1 deletion(-)
#
# Note, a problem with your patch was detected! These changes were made
# automatically for you:
# CARDLIST.saa7134 | 1 +
# 1 files changed, 1 insertion(+)
#
# Please review these changes and see if they belong in your patch.
#
# For better log display, please keep a blank line after subject, after from
# and before signed-off-by
# [and so on]

Something like this would be fine with me. In fact, I think that it is
a great idea!

Here are two patches to do this. The first adjusts the output of the
whitespace cleaning script so the diffstat looks right. The second adds
the text above to the initial commit message.

The prep_commit_msg.pl script is almost totally re-written. This script
will check some more things to try to find the e-mail to use in the commit
message, like scanning the ~/.hgrc file for the username Hg is using.

The Makefile is changed so that temporary files created by make commit and
make whitespace will honor the $TMP environment variable, if set.