Requesting review of complex Xfburn changes
andre at andreldm.com
andre at andreldm.com
Mon Oct 24 12:37:07 CEST 2022
Hi all,
I was wondering the same, I can do code review (it's in my todolist, just don't wait for me to merge), but without an actual burner it's hard to actually test the app. Do you know any mock burner?
Cheers,
Andre Miranda
Oct 24, 2022, 11:31 by trash.paradise at protonmail.com:
> Hi Hunter,
>
> I took a quick look at your MR and it looks good, however I can't say much more as I don't have a burner to test and don't know the Xfburn code.
>
> It builds and installs fine for me, however it then coredumps on launch. But it doesn't seem to be related to this MR, it's also the case with git master, I'll open a new issue about that.
>
> If no one can do a real code review in the near future, which is likely, I think you should go ahead and merge if it's right for you. You can always complete/fix later.
>
> Cheers,
> Gaƫl
>
> ------- Original Message -------
> On Saturday, October 22nd, 2022 at 3:16 AM, Hunter Turcin <huntertur at gmail.com> wrote:
>
>
>> Hello again :)
>>
>> I would like to thank you folks again for letting me volunteer to take
>> on the maintainership of Xfburn. Over the past few days, I've been
>> working on reviewing the long-pending merge requests and fixing some of
>> the long-standing bugs, inconsistencies, and crashes in the
>> application. I also seem to keep finding more of these bugs along the
>> way...
>>
>> During an investigation of one such issue (a GTK assertion failure that
>> occurs when plugging in a burner), I discovered that the bones for
>> device hotplugging support were present in Xfburn and that those bones
>> just needed some massaging in order to let the application be able to
>> respond dynamically to additions and removals of burners instead of
>> only being able to properly respond to disc change events
>> (insertion/removal). In my testing so far, this has led to a more
>> robust Xfburn that is better equipped to handle the user forgetting to
>> plug in a burner (or forgetting to swap out the wrong burner for the
>> right burner) without needing to restart the application.
>>
>> I have more details in the pending merge request (linked below). Please
>> take a look at reviewing it if anyone has the time (or the knowledge);
>> while everything seems to work properly in my testing, it would be good
>> to have some extra pairs of eyes seeing if there's anything missing or
>> any glaring violations of maintainability that might have been
>> introduced.
>>
>> https://gitlab.xfce.org/apps/xfburn/-/merge_requests/23
>>
>> Additionally, am I correct in my assumption that HAVE_GUDEV is only
>> ever going to be defined on a Linux system?
>>
>> Thanks again,
>> Hunter Turcin
>> _______________________________________________
>> Xfce4-dev mailing list
>> Xfce4-dev at xfce.org
>> https://mail.xfce.org/mailman/listinfo/xfce4-dev
>>
> _______________________________________________
> Xfce4-dev mailing list
> Xfce4-dev at xfce.org
> https://mail.xfce.org/mailman/listinfo/xfce4-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.xfce.org/pipermail/xfce4-dev/attachments/20221024/2bb9e91f/attachment.html>
More information about the Xfce4-dev
mailing list