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.

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

fondo, notes-up: init #56172

Merged
merged 3 commits into from Feb 23, 2019
Merged

fondo, notes-up: init #56172

merged 3 commits into from Feb 23, 2019

Conversation

@worldofpeace
Copy link
Member

worldofpeace commented Feb 22, 2019

Motivation for this change

@davidak wanted notes-up.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

davidak left a comment

Looks good.

Thank you very much!

@dtzWill

This comment has been minimized.

Copy link
Contributor

dtzWill commented Feb 22, 2019

FWIW when trying to run this, it seems to bail (I'm not using full pantheon bits):

$ com.github.philip-scott.notes-up                                                                     12:12:34 on 19-02-22
[INFO 12:12:36.547813] Application.vala:155: Notes-Up version: 1.3
[INFO 12:12:36.547849] Application.vala:157: Kernel version: 4.20.11
[FATAL 12:12:36.692385] file /build/source/src/Widgets/Headerbar.vala: line 126: uncaught error: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: The name org.elementary.Contractor was not provided by any .service files (g-dbus-error-quark, 2)
[FATAL 12:12:36.723962] [Gtk] gtk_widget_get_parent: assertion 'GTK_IS_WIDGET (widget)' failed
[FATAL 12:12:36.724005] [Gtk] gtk_widget_freeze_child_notify: assertion 'GTK_IS_WIDGET (widget)' failed
zsh: segmentation fault (core dumped)  com.github.philip-scott.notes-up

Is this something to be resolved at the packaging level, or more intrinsic fault regarding compatibility in other environments?

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Feb 22, 2019

I'm pretty sure fondo would only ever be useful in pantheon because of their greeter setup, etc.

But I think for notes-up that there's a special build flag that disables contractor support, because outside pantheon you won't have that interface.

So i probably should add a subpackage maybe that has -Dnoele=1 in cmakeFlags.

@dtzWill

This comment has been minimized.

Copy link
Contributor

dtzWill commented Feb 22, 2019

So i probably should add a subpackage maybe that has -Dnoele=1 in cmakeFlags.

As you see fit, and thank you. I mostly wanted to be sure it didn't indicate a dep that needed to be propagated or otherwise was a packaging bug.

Sounds like it's good as -is :).

@dtzWill

This comment has been minimized.

Copy link
Contributor

dtzWill commented Feb 22, 2019

Sorry for hijacking this a bit, but just FYI for anyone with this sort of issue:

Simply adding pantheon.contractor.enable = true; seems to have done the trick, no more crashing and doesn't seem to be intrusive or require full buy-in. But I wouldn't expect too much re:functionality since it likely is meant for other components too. Anyway, thanks!

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Feb 22, 2019

Sorry for hijacking this a bit, but just FYI for anyone with this sort of issue:

Simply adding pantheon.contractor.enable = true; seems to have done the trick, no more crashing and doesn't seem to be intrusive or require full buy-in. But I wouldn't expect too much re:functionality since it likely is meant for other components too. Anyway, thanks!

No hijack done 😄 this should work wherever by default 366945a

@dtzWill

This comment has been minimized.

Copy link
Contributor

dtzWill commented Feb 22, 2019

❤️ thanks! Yes this looks great!

@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Feb 22, 2019

@worldofpeace worldofpeace force-pushed the worldofpeace:elementary-post branch from 366945a to 6612b0b Feb 22, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Feb 22, 2019

ping @jtojnar

@jtojnar

This comment has been minimized.

Copy link
Contributor

jtojnar commented Feb 22, 2019

Looks about right.

This adds pantheon.notes-up which will only work in pantheon.
@worldofpeace worldofpeace force-pushed the worldofpeace:elementary-post branch from 6612b0b to ac2546b Feb 23, 2019
@worldofpeace worldofpeace merged commit 904732d into NixOS:master Feb 23, 2019
1 check was pending
1 check was pending
grahamcofborg-eval Cloning project
Details
@worldofpeace worldofpeace deleted the worldofpeace:elementary-post branch Feb 23, 2019
@worldofpeace

This comment has been minimized.

Copy link
Member Author

worldofpeace commented Feb 23, 2019

Thanks for the reviews everyone 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.