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

jamulus: init at 3.5.3 #87914

Merged
merged 2 commits into from
Jun 4, 2020
Merged

jamulus: init at 3.5.3 #87914

merged 2 commits into from
Jun 4, 2020

Conversation

seb314
Copy link
Contributor

@seb314 seb314 commented May 16, 2020 •
edited
Loading

Motivation for this change

Jamulus did not seem to be packaged for nixos before and it would be great to use it on nixos!
My first nix package, I hope I didn't do anything stupid (works fine on my nixos laptop).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels May 16, 2020
@seb314 seb314 changed the title Submit/jamulus. Enables musicians to perform real-time jam sessions over the internet jamulus: init at 3.5.3 enables musicians to perform real-time jam sessions over the internet May 16, 2020
@seb314 seb314 force-pushed the submit/jamulus branch 2 times, most recently from c604131 to 4ab0220 Compare May 17, 2020 13:56
@seb314 seb314 changed the title jamulus: init at 3.5.3 enables musicians to perform real-time jam sessions over the internet jamulus: init at 3.5.3 May 17, 2020
jtojnar
jtojnar reviewed May 17, 2020
View reviewed changes
@seb314
Copy link
Contributor Author

seb314 commented May 17, 2020

The project' output binary is named "Jamulus" by default. (uppercase J). the documentation mentions the option to set it to lowercase "jamulus" instead

To use this file configure the software with qmake "CONFIG+=noupcasename" Jamulus.pro to make sure the output target name of this software is jamulus instead of Jamulus.

should we do that?

@jtojnar
Copy link
Member

jtojnar commented May 17, 2020

Uppercase binary name is fine.

seb314 added a commit to seb314/jamulus that referenced this pull request May 18, 2020
@seb314
optional INSTALL_PATH qmake flag (on unix)
278925e 
I packaged Jamulus for nix/nixos and it would
be neat if there was an option to specify a
path at which to install the binary. I'm not
really familiar with qmake -- is this a good
way to achieve this?

(nixpkgs pull request: NixOS/nixpkgs#87914)
@seb314 seb314 mentioned this pull request May 18, 2020
Merged
@seb314
Copy link
Contributor Author

seb314 commented Jun 2, 2020 •
edited
Loading

Does anything else need to be done before this can be merged?
(Probably combining all but the first commit into one, with title "jamulus: init at 3.5.5")

I'd suggest to postpone the considerations about additional installation of .desktop files and icons, until a clean solution existis in jamulus. They aren't necessary for core funcionality anyway and would increase the complexity here by quite a bit.

@jtojnar
Copy link
Member

jtojnar commented Jun 2, 2020

Squashing into two commits

  • maintainers: add seb314
  • jamulus: init at 3.5.5

should be enough.

@seb314
Copy link
Contributor Author

seb314 commented Jun 2, 2020

Squashing into two commits

* maintainers: add seb314

* jamulus: init at 3.5.5

should be enough.

That can be done by the person who does the merge in basically one click, and doesn't require any additional force-pushes here, right?

@jtojnar
Copy link
Member

jtojnar commented Jun 2, 2020

GitHub only allows squishing everything into a single commit, as far as I am aware. Local interactive rebase and force push will be needed.

@jtojnar jtojnar merged commit 384cf87 into NixOS:master Jun 4, 2020
@jtojnar
Copy link
Member

jtojnar commented Jun 4, 2020

Thanks.

@seb314
Copy link
Contributor Author

seb314 commented Jun 4, 2020

Thanks.

Thanks for the guidance!

@seb314 seb314 deleted the submit/jamulus branch April 17, 2021 10:11
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@seb314 @jtojnar @Janik-Haag