all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Marius Bakke <marius@gnu.org>
To: phodina <phodina@protonmail.com>
Cc: maxim.cournoyer@gmail.com, ludo@gnu.org, maximedevos@telenet.be,
	mail@brendan.scot, leo@famulari.name,
	marekpasnikowski@protonmail.com, h.goebel@crazy-compilers.com,
	57608@debbugs.gnu.org
Subject: [bug#57608] Upstreaming KDE Plasma and rest of packages
Date: Tue, 22 Nov 2022 11:56:38 +0100	[thread overview]
Message-ID: <87wn7nz28p.fsf@gnu.org> (raw)
In-Reply-To: <RkgWytuZm52T0xhoxjTig5b2ALJRL9d4itDBimjK77SiJpkUpv6Va6XMatmT5QUwhxLRYRNQvQcyUN3kgD8OC1Nh95rX3jA8c2afW6Z1YH8=@protonmail.com>

[-- Attachment #1: Type: text/plain, Size: 3738 bytes --]

phodina <phodina@protonmail.com> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

  reply	other threads:[~2022-11-22 10:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-06  7:14 [bug#57608] Upstreaming KDE Plasma and rest of packages phodina via Guix-patches via
2022-09-19 17:50 ` phodina via Guix-patches via
2022-09-24 13:12   ` Ludovic Courtès
2022-09-24 13:15   ` Ludovic Courtès
2022-10-02 17:14     ` guix-patches--- via
2022-10-04  6:52       ` phodina via Guix-patches via
2022-10-04 11:07         ` guix-patches--- via
2022-10-04 14:01           ` phodina via Guix-patches via
2022-10-06 12:09             ` phodina via Guix-patches via
2022-10-06 18:29               ` kiasoc5 via Guix-patches via
2022-10-07 12:57                 ` phodina via Guix-patches via
2022-10-07 14:09                   ` Maxim Cournoyer
2022-10-10 13:03                     ` guix-patches--- via
2022-11-21 21:11               ` Marius Bakke
2022-11-22 10:20                 ` phodina via Guix-patches via
2022-11-22 10:56                   ` Marius Bakke [this message]
2022-12-27 21:15                     ` phodina via Guix-patches via
2022-12-28  0:48                       ` Maxim Cournoyer
2022-10-29 11:59 ` Brendan Tildesley
2022-10-29 20:32   ` guix-patches--- via
2022-10-29 23:49     ` Brendan Tildesley
2022-10-31  7:42     ` phodina via Guix-patches via
2023-09-02  4:23 ` bug#57608: " 宋文武 via Guix-patches via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wn7nz28p.fsf@gnu.org \
    --to=marius@gnu.org \
    --cc=57608@debbugs.gnu.org \
    --cc=h.goebel@crazy-compilers.com \
    --cc=leo@famulari.name \
    --cc=ludo@gnu.org \
    --cc=mail@brendan.scot \
    --cc=marekpasnikowski@protonmail.com \
    --cc=maxim.cournoyer@gmail.com \
    --cc=maximedevos@telenet.be \
    --cc=phodina@protonmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.