phodina skriver: > Hi Marius, > > Thanks for the review and push to master! > >> Awesome work Petr. :-) >> >> I went through the branch and applied most of the patches. With a few >> changes: >> >> * shortened multi-line URLs > > Sorry I used the formatter ('guix style') and it set the URLs as such). Right. It still has a few quirks that needs to be inched out... >> * added (file-name ...) for git sources >> * removed knetworkmanager -- AFAICT it is identical to networkmanager-qt >> * switched to git-fetch for packages that were downloading tarballs from >> the KDE GitLab. This is because such autogenerated tarballs are not >> stable: they may get regenerated with a different hash. > > Is this issue somewhere described? I've lately used more tarballs to git download on other packages. Is the issue connected with any Gitlab/Forge or just the KDE and the release scripts? We have this problem with all the forges. There have been many-a bug report about it, and the linter also warns about it (apparently for GitHub only, that could be improved). >> * dropped the !! commits >> * Minor tweaks to synopses and descriptions > > My apologies as I noticed there were few packages with wrong license or missing description. Oh, I did not license audit these packages as I assumed all the KDE stuffs were using the same license. Can you submit patches to update the licenses? >> * A few commits had a random edit to a different package: I reverted >> those edits. An example: >> https://github.com/phodina/guix/commit/5eaa9c49a78eed419db7847668a55c079bad5b71 > > Caused due to rebasing and working on a large patchset. I'll split it next time and sent it by smaller parts. No worries; this is what reviews are for. :-) >> * Removed use of direct variable references, i.e. #$qtbase. Always use >> (search-input-file ...), (search-input-directory ...) or as a last >> resort #$(this-package-input "foo"). > > Thanks for the hint. I'll fix all the packages where I use this syntax! > >> * Skipped commits that would trigger a lot of rebuilds, e.g. gpgme. > > I'll submit it in new ticket. Note: I added a new version of gpgme and used it for the packages that require it. We already have the newest version on 'core-updates'. >> * Skipped cosmetic commits such as using G-expressions in Qt packages; >> mainly because of rebuilds, but also because they were not indented >> properly. Some also introduced direct #$variable references. > > I'll fix the issues and submit it as a new ticket. Great. >> * Dropped the plasma-desktop-service since it was not working for me. > > Should we keep this ticket open to package also the plasma-desktop-service? > I'll try it now and see what causes the Plasma not to work. Let's keep this ticket open until we get past the finish line. :-) >> For later reference, when adding patches, please add a sentence or two >> at the top of the patch describing what it does. I did not edit the >> patches (except for a long file name), because I did not know what it >> was for. Presumably you did; future you will thank you! >> >> Pushed to 'master' as fe3be8d5e0..82804298ad ! > > Sorry I didn't intend to add so many changes and it's been a challenge that taught me some new lessons. It's easy to get lost in a huge patchset. You did great! > I'll definitely add more description for the reviewers and keep it smaller so I doesn't take long time to review. Bite-sized PRs are more likely to get through, but I'm not sure we had a lot of choice here given the scope of the project. Ultimately someone just needs to muster enough courage to push it forward.