Hi Coiby,
Thanks for the info!
BTW, for this patch set, I have reviewed patch 1~8, which are LGTM.
You can add
Reviewed-by: Tao Liu <ltao(a)redhat.com>
to these patches. So I don't need to send the signature one by one patch.
I will look into the remaining patches 9~15 tomorrow.
Thanks,
Tao Liu
On Mon, Feb 21, 2022 at 5:41 PM Coiby Xu <coxu(a)redhat.com> wrote:
On Mon, Feb 21, 2022 at 04:19:22PM +0800, Tao Liu wrote:
>Hi Coiby,
>
>On Mon, Feb 21, 2022 at 3:41 PM Coiby Xu <coxu(a)redhat.com> wrote:
>>
>> Hi Tao,
>>
>> On Mon, Feb 21, 2022 at 03:09:24PM +0800, Tao Liu wrote:
>> >Hi Philipp,
>> >
>> >I noticed there are ^M characters after the lines which you modified
>> >from this patch set.
>> >
>> >For example in the first patch:
>> >
>> >-StandardOutput=syslog
>> >-StandardError=syslog+console
>> >+StandardOutput=journal^M
>> >+StandardError=journal+console^M
>> >
>> >I think the ^M chars are unnecessary, and should be removed.
>>
>> This should be a know issue [1].
>>
>hmm... Interesting, it looks like an email-mutt issue right?
It's likely an email relay server adds these trailing ^Ms.
>
>Just for curiosity, Will you merge the patches without any modification
>on the patch set or will you use tools like dox2unix to strip these chars
>before the merge?
I use a tool [2] from Kairui to remove trailing ^Ms before applying them.
But that tool can process one patch one time.
[2]
https://gitlab.cee.redhat.com/kasong/dot-home/-/blob/master/misc/format-p...
>
>I didn't use git am much, this will be the first time I encountered this, if
>the patches are OK , then please ignore my comments...
>
>Thanks,
>Tao Liu
>
>> It would be good for Phillipp to push the changes to a remote repo
>> instead.
>>
>> [1]
https://pagure.io/fedora-infrastructure/issue/9831
>> >
>> >Btw, the checkpatch.pl tool gives the similar results on my machine:
>> >
>> >ERROR: DOS line endings
>> >#32: FILE: dracut-kdump-capture.service:23:
>> >+StandardOutput=journal^M$
>> >
>> >ERROR: DOS line endings
>> >#33: FILE: dracut-kdump-capture.service:24:
>> >+StandardError=journal+console^M$
>> >
>> >I don't know if it is caused by your editor or my mutt program when
downloading
>> >the patches.
>> >
>> >As for the other patches, I'm still reviewing them, please wait for a
while.
>> >
>> >Thanks,
>> >Tao Liu
>> >
>> >
>> >On Sat, Feb 5, 2022 at 1:34 AM Philipp Rudo <prudo(a)redhat.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> while looking into transforming mkdumprd and mkfadumprd into library
functions
>> >> I noticed various nits in kdumpctl. This series addresses them.
>> >>
>> >> The series is made up of two parts:
>> >>
>> >> Patches 1-8 are small independent cleanups and fixes.
>> >>
>> >> Patches 9-15 tries to reduce the number of file accesses to
/etc/kdump.conf. It
>> >> achieves this by only parsing kdump.conf once in check_config and
storing the
>> >> parsed values in an array. Later accesses can then simply use the value
stored
>> >> in the array instead of calling kdump_get_conf_val.
>> >>
>> >> Thanks
>> >> Philipp
>> >>
>> >> Philipp Rudo (15):
>> >> kdump-capture.service: switch to journal for stdout
>> >> kdumpctl: source dependencies before defining globals
>> >> kdump-lib: fix typo in variable name
>> >> kdumpctl: remove unnecessary uses of $?
>> >> kdump-lib-initramfs: merge definitions for default ssh key
>> >> kdumpctl: fix comment in check_and_wait_network_ready
>> >> kdumpctl: forbid aliases from ssh config
>> >> kdumpctl: simplify propagate_ssh_key
>> >> kdumpctl: merge check_ssh_config into check_config
>> >> kdumpctl: reduce file operations on kdump.conf
>> >> kdumpctl: drop SAVE_PATH variable
>> >> kdumpctl: drop SSH_KEY_LOCATION variable
>> >> kdumpctl: drop DUMP_TARGET variable
>> >> kdumpctl: remove kdump_get_conf_val in save_raw
>> >> kdumpctl: simplify local_fs_dump_target
>> >>
>> >> dracut-kdump-capture.service | 4 +-
>> >> dracut-kdump.sh | 2 +-
>> >> kdump-lib-initramfs.sh | 1 +
>> >> kdump-lib.sh | 14 +-
>> >> kdumpctl | 258
++++++++++++++++-------------------
>> >> mkdumprd | 2 +-
>> >> 6 files changed, 128 insertions(+), 153 deletions(-)
>> >>
>> >> --
>> >> 2.34.1
>> >> _______________________________________________
>> >> kexec mailing list -- kexec(a)lists.fedoraproject.org
>> >> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>> >> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>> >> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> >> List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
>> >> Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
>> >_______________________________________________
>> >kexec mailing list -- kexec(a)lists.fedoraproject.org
>> >To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>> >Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>> >List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> >List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
>> >Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
>>
>> --
>> Best regards,
>> Coiby
>>
>
--
Best regards,
Coiby