pgsidekick: init at unstable-2014-11-23 #73720

ggPeti · 2019-11-18T22:43:14Z

Motivation for this change

pglisten is a minimal program to listen to postgresql notifications, and it had no nixpkg yet.

Things done

aanderse

The package looks simple enough. I appreciate the test, though I'm not sure what the policy on having tests for applications is... I'm under the impression that usually we don't have tests for applications unless we have specific reasons to do so... I bet @worldofpeace would know the answer to that 😄

Maybe @danbst, being one of our resident postgres experts, could give this PR a quick look over, but I think after we hear back on the testyou might just drop it (or keep it?) and we're good to go.

BTW if you end up keeping the test you'll need to add the test to the list of all tests so our bot can run it.

ggPeti · 2019-11-22T11:34:51Z

Cheers for the review, I'm ready to drop the test if that's the verdict.

worldofpeace · 2019-11-22T11:49:08Z

The package looks simple enough. I appreciate the test, though I'm not sure what the policy on having tests for applications is... I'm under the impression that usually we don't have tests for applications unless we have specific reasons to do so... I bet @worldofpeace would know the answer to that smile

Maybe @danbst, being one of our resident postgres experts, could give this PR a quick look over, but I think after we hear back on the testyou might just drop it (or keep it?) and we're good to go.

BTW if you end up keeping the test you'll need to add the test to the list of all tests so our bot can run it.

A test is useful at the discretion of the maintainer. So I think if the maintainer has put the effort to have some sort of testing, 👍 to them. It'd make verifying the installation of the utilities much easier.
It's true the imperative vm tests are pretty expensive for applications, but in this case you can't verify the function without a postgresql service.

It's also not the only test for applications we have

Something like #36842 would be better for this though.
(but in this instance it does need a nixos service anyway).

worldofpeace

Your package expression is missing proper whitespace, see most other expressions in nixpkgs.

{ stdenv }:

stdenv.mkDerivation { 
  pname = ...
  version = ...

  src = ...

  buildInputs = ...

  installPhase = ''...
  '';

  meta = {
  # these aren't usually separated by whitespace
  }
}

worldofpeace · 2019-11-22T11:52:43Z

nixos/tests/pgsidekick.nix

+        CREATE TRIGGER test_trg AFTER INSERT ON test FOR EACH ROW EXECUTE PROCEDURE new_row_notify();
+      '';
+    };
+    systemd.services.listener = {


Suggested change

systemd.services.listener = {

systemd.services.listener = {

worldofpeace · 2019-11-22T11:55:25Z

The package looks simple enough. I appreciate the test, though I'm not sure what the policy on having tests for applications is... I'm under the impression that usually we don't have tests for applications unless we have specific reasons to do so... I bet @worldofpeace would know the answer to that smile
Maybe @danbst, being one of our resident postgres experts, could give this PR a quick look over, but I think after we hear back on the testyou might just drop it (or keep it?) and we're good to go.
BTW if you end up keeping the test you'll need to add the test to the list of all tests so our bot can run it.

A test is useful at the discretion of the maintainer. So I think if the maintainer has put the effort to have some sort of testing, +1 to them. It'd make verifying the installation of the utilities much easier.
It's true the imperative vm tests are pretty expensive for applications, but in this case you can't verify the function without a postgresql service.

It's also not the only test for applications we have
* https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/signal-desktop.nix

* https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/firefox.nix
Something like #36842 would be better for this though.
(but in this instance it does need a nixos service anyway).

Then again, this repo hasn't been touched since it was published. So the test wouldn't be useful for verifying updates, just continued function with our other build expressions (because they will change).

ggPeti · 2019-11-22T12:10:32Z

pgsidekick is built against whatever postgresql this package is called with, that will change in the future. Is there any caching mechanism for VM tests so they don't run again unless something changes?

worldofpeace · 2019-11-22T13:22:32Z

pgsidekick is built against whatever postgresql this package is called with, that will change in the future. Is there any caching mechanism for VM tests so they don't run again unless something changes?

I believe if you're on one rev of nixpkgs and you've built the test successfully it will return the cached output. It also seems you've forgotten to add your test to all-tests.nix, which should give it a job on hydra.

danbst · 2019-12-09T10:47:17Z

@ggPeti we'd merge the package (even that it is unmaintained), but can you move the test into pkgs/tools/misc/pgsidekick directory? We haven't yet created guides on how to do package testing, and even if we will have ones, the test for a package should live closer to package itself.

worldofpeace · 2019-12-09T10:52:27Z

@ggPeti we'd merge the package (even that it is unmaintained), but can you move the test into pkgs/tools/misc/pgsidekick directory? We haven't yet created guides on how to do package testing, and even if we will have ones, the test for a package should live closer to package itself.

I thought we decided on add an attr in passthru

passthru.tests = { 
  pgsidekick = nixosTests.pgsidekick;
};

at least, that's what I've been doing in https://github.com/NixOS/nixpkgs/tree/master/nixos/tests/installed-tests.
We even documented it https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes passthru.tests.

danbst · 2019-12-09T11:23:59Z

@worldofpeace oh didn't know about installed-tests! Looks like a sane approach. I agree about adding passthru.tests as you've described.

@ggPeti would you like to finish this up?

danbst · 2020-04-03T09:02:08Z

pkgs/servers/sql/postgresql/ext/pgsql_http.nix

+  '';
+
+  meta = with stdenv.lib; {
+    description = "Temporal Tables PostgreSQL Extension ";


Looks like description is copy-pasted from other expression, right?

stale · 2020-09-30T23:01:34Z

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

pkgs/tools/misc/pgsidekick/default.nix

pkgs/servers/sql/postgresql/ext/pgsql_http.nix

SuperSandro2000 · 2021-01-18T17:27:27Z

pkgs/servers/sql/postgresql/ext/pgsql_http.nix

+  buildInputs = [ curl postgresql ];
+
+ src = fetchGit {
+    url = "git://github.com/pramsey/pgsql-http";


Please use fetchFromGitHub.

why? fetchGit is a builtin, what's wrong with using it?

stale · 2021-07-21T02:11:52Z

I marked this as stale due to inactivity. → More info

ofborg bot added 6.topic: nixos 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Nov 18, 2019

aanderse reviewed Nov 22, 2019

View reviewed changes

worldofpeace suggested changes Nov 22, 2019

View reviewed changes

worldofpeace reviewed Nov 22, 2019

View reviewed changes

ggPeti force-pushed the master branch from ac71ddf to f987629 Compare March 29, 2020 19:27

ggPeti requested a review from thoughtpolice as a code owner March 29, 2020 19:27

ggPeti force-pushed the master branch 2 times, most recently from c33f55c to fb9b0a5 Compare March 29, 2020 19:46

danbst reviewed Apr 3, 2020

View reviewed changes

stale bot added the 2.status: stale label Sep 30, 2020

SuperSandro2000 requested changes Jan 18, 2021

View reviewed changes

stale bot removed the 2.status: stale label Jan 18, 2021

stale bot added the 2.status: stale label Jul 21, 2021

ggPeti closed this Aug 15, 2021

ggPeti force-pushed the master branch from 4fef322 to 3ddf009 Compare August 15, 2021 14:39

github-actions bot removed the 6.topic: nixos label Aug 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pgsidekick: init at unstable-2014-11-23 #73720

pgsidekick: init at unstable-2014-11-23 #73720

ggPeti commented Nov 18, 2019

aanderse left a comment

ggPeti commented Nov 22, 2019

worldofpeace commented Nov 22, 2019

worldofpeace left a comment •

edited

Loading

worldofpeace Nov 22, 2019

worldofpeace commented Nov 22, 2019

ggPeti commented Nov 22, 2019

worldofpeace commented Nov 22, 2019

danbst commented Dec 9, 2019

worldofpeace commented Dec 9, 2019

danbst commented Dec 9, 2019

danbst Apr 3, 2020

stale bot commented Sep 30, 2020

SuperSandro2000 Jan 18, 2021

ggPeti Jan 18, 2021

stale bot commented Jul 21, 2021

pgsidekick: init at unstable-2014-11-23 #73720

pgsidekick: init at unstable-2014-11-23 #73720

Conversation

ggPeti commented Nov 18, 2019

Motivation for this change

Things done

aanderse left a comment

Choose a reason for hiding this comment

ggPeti commented Nov 22, 2019

worldofpeace commented Nov 22, 2019

worldofpeace left a comment • edited Loading

Choose a reason for hiding this comment

worldofpeace Nov 22, 2019

Choose a reason for hiding this comment

worldofpeace commented Nov 22, 2019

ggPeti commented Nov 22, 2019

worldofpeace commented Nov 22, 2019

danbst commented Dec 9, 2019

worldofpeace commented Dec 9, 2019

danbst commented Dec 9, 2019

danbst Apr 3, 2020

Choose a reason for hiding this comment

stale bot commented Sep 30, 2020

SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

ggPeti Jan 18, 2021

Choose a reason for hiding this comment

stale bot commented Jul 21, 2021

worldofpeace left a comment •

edited

Loading