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

twinkle: init at 1.10.2 #74767

Merged
merged 2 commits into from Dec 7, 2019
Merged

twinkle: init at 1.10.2 #74767

merged 2 commits into from Dec 7, 2019

Conversation

@mkg20001
Copy link
Member

mkg20001 commented Dec 1, 2019

Motivation for this change

Need this for myself, useful for others as well.

Things done

Added the twinkle package

  • 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 nix-review --run "nix-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.
Notify maintainers
@rnhmjoj

This comment has been minimized.

Copy link
Contributor

rnhmjoj commented Dec 1, 2019

Could you please follow the commit message convention?

Copy link
Contributor

c0bw3b left a comment

Also the separate commit to add yourself to maintainers-list should read
maintainers: add mkg20001

@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch 2 times, most recently from 367c441 to 5f98e27 Dec 1, 2019
@mkg20001 mkg20001 changed the title twinkle: added 1.10.2 twinkle: init at 1.10.2 Dec 1, 2019
@mkg20001 mkg20001 requested a review from c0bw3b Dec 7, 2019
@mkg20001

This comment has been minimized.

Copy link
Member Author

mkg20001 commented Dec 7, 2019

Can this get merged please?

@rnhmjoj
rnhmjoj approved these changes Dec 7, 2019
Copy link
Contributor

rnhmjoj left a comment

Looks good now.

@nixos-discourse

This comment has been minimized.

Copy link

nixos-discourse commented Dec 7, 2019

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/78

Copy link
Member

timokau left a comment

When you think the PR is mergeable, please squash the commit history into two commits (one adding yourself as a maintainer, one adding the package).

There is no need to maintain commit history within a PR. Commits should be logical units that could be useful in the future, e.g. for a revert, a bisect, a cherry-pick. For example it might make sense to revert the package addition without also reverting your addition to the maintainers list. But there is no reason to keep fixup commits like "twinkle: fix description".


nativeBuildInputs = [ cmake bison flex wrapQtAppsHook bcg729 ];

cmakeFlags = [ "-DWITH_G729=On" "-DWITH_SPEEX=On" "-DWITH_ILBC=On" /* "-DWITH_DIAMONDCARD=On" broken code */ ];

This comment has been minimized.

Copy link
@timokau

timokau Dec 7, 2019

Member

What does "broken code" mean?

This comment has been minimized.

Copy link
@mkg20001

mkg20001 Dec 7, 2019

Author Member

Seems ancient and broken

/build/source/src/gui/mphoneform.cpp: In member function 'void MphoneForm::init()':
/build/source/src/gui/mphoneform.cpp:244:9: error: 'class QMenu' has no member named 'insertItem'; did you mean 'insertMenu'?
   menu->insertItem("Diamondcard", Diamondcard);
         ^~~~~~~~~~
         insertMenu
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from 0dbf936 to 5b6f815 Dec 7, 2019
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from 5b6f815 to f681f50 Dec 7, 2019
@mkg20001 mkg20001 requested a review from timokau Dec 7, 2019
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch 2 times, most recently from 5238df3 to 7eb798f Dec 7, 2019
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from 7eb798f to 89d62ba Dec 7, 2019
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch 3 times, most recently from e6ea734 to eb71e9b Dec 7, 2019

cmakeFlags = [ "-DWITH_G729=On" "-DWITH_SPEEX=On" "-DWITH_ILBC=On" /* "-DWITH_DIAMONDCARD=On" seems ancient and broken */ ];

enableParallelBuilding = true;

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Dec 7, 2019

Member

this is default 031138b, no need to specify

This comment has been minimized.

Copy link
@mkg20001

mkg20001 Dec 7, 2019

Author Member

But only for cmake and not meson?

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Dec 7, 2019

Member

Ah, the commit I mentioned is for cmake which this project uses. For meson it's for sure a default.

@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from eb71e9b to a24e24d Dec 7, 2019
Copy link
Member

worldofpeace left a comment

LGTM. Could you add a changelog meta attribute?

changelog = "https://github.com/LubosD/twinkle/blob/${version}/NEWS";
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from a24e24d to 418b5f4 Dec 7, 2019
@timokau
timokau approved these changes Dec 7, 2019
Copy link
Member

timokau left a comment

Thanks!

@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from 418b5f4 to a24e24d Dec 7, 2019
@mkg20001 mkg20001 force-pushed the mkg20001:pkg/twinkle branch from a24e24d to 367058a Dec 7, 2019
@mkg20001

This comment has been minimized.

Copy link
Member Author

mkg20001 commented Dec 7, 2019

So, somewhere in between I messed up and pushed an older version

Fixed that

@timokau timokau merged commit 07266e2 into NixOS:master Dec 7, 2019
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
twinkle on aarch64-linux Success
Details
twinkle on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.