GNU bug report logs - #68401
30.0.50; ERC 5.6-git: `erc-cmd-GMSG', `erc-cmd-AMSG', `erc-cmd-GME', `erc-cmd-AME'. 2nd attempt

Previous Next

Full log

View this message in rfc822 format

From: Emanuel Berg <incal <at> dataswamp.org>
To: 68401 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: bug#68401: 30.0.50; ERC 5.6-git: `erc-cmd-GMSG', `erc-cmd-AMSG', `erc-cmd-GME', `erc-cmd-AME'. 2nd attempt
Date: Sun, 11 Feb 2024 02:58:11 +0100

J.P. wrote:

>> * lisp/erc/erc.el (erc-cmd-AMSG): Make it consistent with the doc
>> string by only affecting the current connection.
>> (erc-cmd-GMSG, erc-cmd-AME, erc-cmd-GME): new IRC slash commands
>> (Bug#68401)
>
> Looks like you've combined ("fixed up") your patch with the
> provided test and data files. That's fine, but its changes
> should be listed in the commit message as well:
>
>   * test/lisp/erc/erc-scenarios-misc-commands.el
>   (erc-scenarios-misc-commands--AMSG-GMSG-AME-GME): New test.
>   * test/lisp/erc/resources/commands/amsg-barnet.eld: New file.
>   * test/lisp/erc/resources/commands/amsg-foonet.eld: New file.

Should those lines be added to the commit message? I was
unaware of those messages, if they have appeared previously.
Okay, I do that with 'git commit --amend', right?

> In the future, maybe look into "squashing" to
> preserve messages.

I think that was what I did, using the following commands.
I wrote them down after I did it, so maybe someone is missing.
ib is my local branch.

$ git log --graph --oneline --decorate -a
$ git rebase -i HEAD~5
$ git commit --amend
$ git rebase --onto origin/master 'ib^' ib
$ git pull
$ git format-patch master -M -o .ib

I was told there was an easier way, using diff, since all that
was called for was a commit that expressed the difference from
the initial checkout to the current state of the source -
ignoring the detour back and forth and their corresponding
commits. So maybe there is an easier way than all
those commands?

> Also, consider adding an entry to etc/ERC-NEWS if you think
> people can benefit from these commands.

OK.

>> +      (erc--connected-and-joined-p)
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Seems one of these things is not like the others.
> Guessing that's unintentional.

Ah, you are right, good that you saw it. If I change that in
the source to the correct #'erc--connected-and-joined-p and
commit that, we are back at the previous situation. Only now
we have 2 commits instead of 5.

Should I change the code, commit the change, and do the whole
rebase stuff again to get a single patch?

Or is there an easier way?

> What's not great is that the test still passes in spite of
> this. It seems /GME is the only variant not covered, which
> I guess is my fault. Perhaps you should improve the test so
> it fails with the current patch applied and passes once
> it's fixed.

I'm not familiar with those tests so it is better you do
that part, I think.

> Is this FIXME comment regarding your paperwork accurate?

That had to do with my package on GNU ELPA [1] but that
package has appeared so I suppose the paperwork issue has
been solved.

[1] https://dataswamp.org/~incal/emacs-init/wrap-search.el

-- 
underground experts united
https://dataswamp.org/~incal

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.