kitsas: init at 2.3 #115270

gspia · 2021-03-06T16:36:44Z

kitsas: init at 2.3

Motivation for this change

This adds an accounting application called kitsas to the set of available applications. Kitsas is suitable for Finnish associations and small business.

This is my first packaged application and related pull request. (I have no exp on reviewing nixpkgs issues yet.)

Things done

r-rmcgibbo · 2021-03-06T17:04:11Z

Result of nixpkgs-review pr 115270 at 5cb46d27 run on aarch64-linux 1

1 package built:

kitsas

4 suggestions:

warning: missing-phase-hooks
configurePhase should probably contain runHook preConfigure and runHook postConfigure.

Near pkgs/applications/office/kitsas/default.nix:20:3:
```
   |
20 |   configurePhase = ''
   |   ^
```
warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/office/kitsas/default.nix:29:3:
```
   |
29 |   installPhase = ''
   |   ^
```
warning: unclear-gpl
gpl3 is a deprecated license, check if project uses gpl3Plus or gpl3Only and change meta.license accordingly.

Near pkgs/applications/office/kitsas/default.nix:43:5:
```
   |
43 |     license = lib.licenses.gpl3;
   |     ^
```

warning: unused-argument

Unused argument: qtcreator.
Near pkgs/applications/office/kitsas/default.nix:1:53:

  |
1 | { lib, mkDerivation, fetchFromGitHub, qmake, qtsvg, qtcreator, libsForQt5, libzip, pkg-config  }:
  |                                                     ^

Result of nixpkgs-review pr 115270 at 5cb46d27 run on x86_64-linux 1

1 package built:

kitsas

4 suggestions:

warning: missing-phase-hooks
configurePhase should probably contain runHook preConfigure and runHook postConfigure.

Near pkgs/applications/office/kitsas/default.nix:20:3:
```
   |
20 |   configurePhase = ''
   |   ^
```
warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/office/kitsas/default.nix:29:3:
```
   |
29 |   installPhase = ''
   |   ^
```
warning: unclear-gpl
gpl3 is a deprecated license, check if project uses gpl3Plus or gpl3Only and change meta.license accordingly.

Near pkgs/applications/office/kitsas/default.nix:43:5:
```
   |
43 |     license = lib.licenses.gpl3;
   |     ^
```

warning: unused-argument

Unused argument: qtcreator.
Near pkgs/applications/office/kitsas/default.nix:1:53:

  |
1 | { lib, mkDerivation, fetchFromGitHub, qmake, qtsvg, qtcreator, libsForQt5, libzip, pkg-config  }:
  |                                                     ^

pkgs/applications/office/kitsas/default.nix

pkgs/top-level/all-packages.nix

pkgs/applications/office/kitsas/default.nix

gspia · 2021-03-09T14:18:26Z

Hopefully the commits do what you asked @SuperSandro2000 . Could you please re-review the changes?

pkgs/applications/office/kitsas/default.nix

gspia · 2021-03-10T05:19:03Z

Thanks for the snd round @SuperSandro2000 ! How this MR looks now?

While implementing the changes, I started to wonder if there is a tool available to check the style. And possibly some of the facts like the one that using qmake means enabling parallel building. E.g. something like hlint that is used in Haskell. (I'll probably start a thread at nixos discourse as if there are tools, I think other people would benefit knowing about those - I tried to search for some but didn't find anything yet.)

Late addition:

It turned out that there are several suitable tools. I'll start trying them out.

rmcgibbo · 2021-03-16T14:51:27Z

There are a few formatters (I use nixpkgs-format), and there's also nixpkgs-hammering to give hints. Can you squash the commits down into 2? It should be two commits like "maintainers: add {name}", and then "kitsas: init at 2.3".

gspia · 2021-03-17T05:10:44Z

Haa and thanks!
I hope this has now better shape and sorry for not realizing right in the beginning to squash also the review changes into two commits and using the correctly formatted commit titles.
Thus, could you please check once more @rmcgibbo and/or @SuperSandro2000 that everything is ok?

pkgs/applications/office/kitsas/default.nix

nixos-discourse · 2021-04-07T04:13:04Z

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/497

SuperSandro2000 · 2021-04-07T08:35:49Z

pkgs/applications/office/kitsas/default.nix

+    mkdir -p $out/bin $out/share
+    cp kitsas $out/bin
+    cp $src/kitsas.png $out/share
+    cp $src/kitsas.desktop $out/share


Suggested change

cp $src/kitsas.desktop $out/share

cp $src/kitsas.desktop $out/share/applications

Thanks, I made this change and tried two formatters on this default.nix file and included the changes (the qmakeFlags on separate lines). Further, I found a typo on the description (the associations word).

To be sure that there is that applications directory, I added it into the mkdir command above.

I didn't realize at all that the applications directory was needed (the icons were "working" in my case when I tried this locally). (I also tried to try the hammering but wasn't yet able to see, how to use it.)

SuperSandro2000 · 2021-04-07T08:36:00Z

pkgs/applications/office/kitsas/default.nix

+  installPhase = ''
+    mkdir -p $out/bin $out/share
+    cp kitsas $out/bin
+    cp $src/kitsas.png $out/share


This probably needs adjustment, too.

Ok, I copied this too into the applications directory.

Add kitsas, an accounting application This commit adds an accounting application called kitsas to the set of packages. Kitsas is suitable for Finnish associations and small business. Change meta maintainers Change the meta license line Add newlines Change the top level caller Start using qmakeFlags Second review round changes Change license to non-deprecated one Typo in the license constant

gspia changed the title ~~Add kitsas, an accounting application~~ kitsas: init at 2.3 Mar 6, 2021

ofborg bot added 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Mar 6, 2021

SuperSandro2000 requested changes Mar 6, 2021

View reviewed changes

ofborg bot added the 11.by: package-maintainer label Mar 7, 2021

gspia requested a review from SuperSandro2000 March 8, 2021 04:37

SuperSandro2000 reviewed Mar 9, 2021

View reviewed changes

pkgs/applications/office/kitsas/default.nix Outdated Show resolved Hide resolved

pkgs/applications/office/kitsas/default.nix Outdated Show resolved Hide resolved

pkgs/applications/office/kitsas/default.nix Outdated Show resolved Hide resolved

gspia requested a review from SuperSandro2000 March 10, 2021 05:03

gspia force-pushed the kitsas branch 2 times, most recently from 2bd11d3 to 86131b5 Compare March 17, 2021 05:07

SuperSandro2000 reviewed Mar 18, 2021

View reviewed changes

pkgs/applications/office/kitsas/default.nix Outdated Show resolved Hide resolved

gspia force-pushed the kitsas branch 2 times, most recently from 7ea5b7c to 1edf913 Compare March 22, 2021 05:12

gspia requested a review from SuperSandro2000 March 22, 2021 05:29

maintainers: add {gspia}

ca743d5

gspia force-pushed the kitsas branch from 1edf913 to 9c7e199 Compare April 5, 2021 06:07

SuperSandro2000 reviewed Apr 7, 2021

View reviewed changes

gspia force-pushed the kitsas branch from 9c7e199 to 8d75eaf Compare April 10, 2021 04:58

gspia requested a review from SuperSandro2000 April 10, 2021 07:34

SuperSandro2000 merged commit f1ee447 into NixOS:master Apr 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kitsas: init at 2.3 #115270

kitsas: init at 2.3 #115270

gspia commented Mar 6, 2021 •

edited

r-rmcgibbo commented Mar 6, 2021 •

edited

gspia commented Mar 9, 2021

gspia commented Mar 10, 2021 •

edited

rmcgibbo commented Mar 16, 2021

gspia commented Mar 17, 2021 •

edited

nixos-discourse commented Apr 7, 2021

SuperSandro2000 Apr 7, 2021

gspia Apr 10, 2021 •

edited

SuperSandro2000 Apr 7, 2021

gspia Apr 10, 2021

	cp $src/kitsas.desktop $out/share
	cp $src/kitsas.desktop $out/share/applications

kitsas: init at 2.3 #115270

kitsas: init at 2.3 #115270

Conversation

gspia commented Mar 6, 2021 • edited

Motivation for this change

Things done

r-rmcgibbo commented Mar 6, 2021 • edited

gspia commented Mar 9, 2021

gspia commented Mar 10, 2021 • edited

rmcgibbo commented Mar 16, 2021

gspia commented Mar 17, 2021 • edited

nixos-discourse commented Apr 7, 2021

SuperSandro2000 Apr 7, 2021

Choose a reason for hiding this comment

gspia Apr 10, 2021 • edited

Choose a reason for hiding this comment

SuperSandro2000 Apr 7, 2021

Choose a reason for hiding this comment

gspia Apr 10, 2021

Choose a reason for hiding this comment

gspia commented Mar 6, 2021 •

edited

r-rmcgibbo commented Mar 6, 2021 •

edited

gspia commented Mar 10, 2021 •

edited

gspia commented Mar 17, 2021 •

edited

gspia Apr 10, 2021 •

edited