Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lxqt: fix packaging errors #24896

Closed
wants to merge 2 commits into from
Closed

lxqt: fix packaging errors #24896

wants to merge 2 commits into from

Conversation

tholzschuh
Copy link

the current lxqt expression doesn't work in 17.03 because of the missing 'pkgs'.

@romildo @tholzschuh
lxqt: fix qtsvg attribute
fc172bd
@romildo @tholzschuh
lxqt-panel: fix for explicit (a C++11 keyword) being used as variable
c02ec45 
It is needed to override "explicit" as this is a C++ keyword. But it
is used as variable name in xkb.h. This is causing a failure in C++
compile time.  Similar bug here:
https://bugs.freedesktop.org/show_bug.cgi?id=74080

Workaround from
lxqt/lxqt-panel@ec62109.
@mention-bot
Copy link

@tholzschuh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ttuegel and @romildo to be potential reviewers.

FRidh
FRidh reviewed Apr 14, 2017
View reviewed changes
pkgs/desktops/lxqt/core/lxqt-panel/default.nix
@@ -49,6 +49,13 @@ stdenv.mkDerivation rec {
lxmenu-data
];

patches = [
(fetchurl {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might want to use fetchPatch here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I'm relatively new to nix and I don't really know whether it should be like this or whether I should use fetchPatch. I just took them from master, @romildo wrote the fix..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is a cherry-pick from master, then leave it as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is.

@bjornfor
Copy link
Contributor

Weren't these things fixed on master? If so, please use "git cherry-pick -x" so that we know where it came from (and that it doesn't need to be forward ported to master).

And if this isn't fixed on master, i think we should fix it there first, then backport.

@tholzschuh
Copy link
Author

They are fixed in master, I just wanted to bring it to 17.03.
Ah sorry I missed the "-x". I cherry-picked it from master though, do I still need to reclone the whole thing and cherry-pick -x from master again?

@bjornfor bjornfor self-assigned this Apr 14, 2017
@bjornfor
Copy link
Contributor

I can do the cherry-picks.

@tholzschuh
Copy link
Author

Thank you @bjornfor, looking forward for lxqt :-)

@bjornfor
Copy link
Contributor

Done: 9f64797, 76c6313.

@bjornfor bjornfor closed this Apr 14, 2017
@bjornfor
Copy link
Contributor

do I still need to reclone the whole thing and cherry-pick -x from master again?

For next time, if you remember "cherry-pick -x" just a little too late (i.e. after PR is made):

git reset --hard origin/release-17.03
git cherry-pick -x ...
git push --force

(So no costly re-clone needed.)

@tholzschuh
Copy link
Author

Thank you @bjornfor I'll do it next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjornfor bjornfor bjornfor left review comments

@FRidh FRidh FRidh left review comments

Assignees

@bjornfor bjornfor

Labels
12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tholzschuh @mention-bot @bjornfor @FRidh @Janik-Haag @romildo