Skip to content

migra: init at 3.0.1647431138#253004

Merged
wegank merged 4 commits intoNixOS:masterfrom
soispha:master
Oct 17, 2023
Merged

migra: init at 3.0.1647431138#253004
wegank merged 4 commits intoNixOS:masterfrom
soispha:master

Conversation

@soispha
Copy link
Contributor

@soispha soispha commented Sep 2, 2023 •
edited
Loading

Description of changes

This adds migra, a diff and migration tool for PostgreSQL schemas, closing package request #196732.
Additionally I added both the schemainspect and sqlbag python libraries, as they are requirements for migra and written by the same author.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Sep 2, 2023
@NixOSInfra NixOSInfra added the 12.first-time contribution This PR is the author's first one; please be gentle! label Sep 2, 2023
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 2, 2023
nechtarg
nechtarg suggested changes Sep 10, 2023
View reviewed changes
@nechtarg
Copy link

nechtarg commented Sep 10, 2023

Result of nixpkgs-review pr 253004 run on x86_64-linux 1

10 packages built:
  • migra
  • migra.dist
  • python310Packages.schemainspect
  • python310Packages.schemainspect.dist
  • python310Packages.sqlbag
  • python310Packages.sqlbag.dist
  • python311Packages.schemainspect
  • python311Packages.schemainspect.dist
  • python311Packages.sqlbag
  • python311Packages.sqlbag.dist

pbsds
pbsds requested changes Sep 10, 2023
View reviewed changes
Copy link
Member

@pbsds pbsds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive! Welcome to nixpkgs!

Please view this opinionated guide regarding the order of attributes in your derivation.

It's absolute, but in general you want native build inputs, followed by propagated inputs, then check inputs. As you read the derivation from top to bottom you want the relevant bits to show up in chronological order.

Please also try to switch to the upstream repositories to fetch the tests properly.

@soispha
Copy link
Contributor Author

soispha commented Sep 16, 2023 •
edited
Loading

@pbsds I've applied your suggestions, but I'm struggling with adding the .sql files to the schemainspect's distribution. The line in the sql_import.patch in schemainspect should add them, but it seems like it's ignored

pbsds
pbsds reviewed Sep 16, 2023
View reviewed changes
Copy link
Member

@pbsds pbsds left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that was a deep dive. I think i found the problem: They forgot to enable pep517. Please consider making an issue/pull upstream about it, and link to it next to the fix.

@soispha
Copy link
Contributor Author

soispha commented Sep 26, 2023

@pbsds I had to disable quite a few test as they required a running postgres sever. Is there a way to spin one up, just for the test?

@pbsds
Copy link
Member

pbsds commented Sep 26, 2023

TIL about postgresqlTestHook

@soispha
Copy link
Contributor Author

soispha commented Oct 1, 2023

Okay, I think everything should work now

pbsds
pbsds requested changes Oct 1, 2023
View reviewed changes
@soispha soispha mentioned this pull request Oct 6, 2023
Open
@pbsds
Copy link
Member

pbsds commented Oct 9, 2023 •
edited
Loading

LGTM, good work!

@ofborg build migra

Result of nixpkgs-review pr 253004 run on x86_64-linux 1

10 packages built:
  • migra
  • migra.dist
  • python310Packages.schemainspect
  • python310Packages.schemainspect.dist
  • python310Packages.sqlbag
  • python310Packages.sqlbag.dist
  • python311Packages.schemainspect
  • python311Packages.schemainspect.dist
  • python311Packages.sqlbag
  • python311Packages.sqlbag.dist
  • package path fits guidelines
  • package name fits guidelines
  • package version fits guidelines
  • package build on x86_&4-linux
  • executables tested on x86_&4-linux
  • meta.description is set and fits guidelines
  • meta.license fits upstream license
  • meta.platforms is set
  • meta.maintainers is set
  • build time only dependencies are declared in nativeBuildInputs
  • source is fetched using the appropriate function
  • the list of phases is not overridden
  • when a phase (like installPhase) is overridden it starts with runHook preInstall and ends with runHook postInstall.
  • patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed
  • patches that are remotely available are fetched rather than vendored

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 9, 2023
nechtarg
nechtarg approved these changes Oct 11, 2023
View reviewed changes
Copy link

@nechtarg nechtarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm obviously not as handy as @pbsds when it comes to reviewing nix (especially nixpkgs) related stuff, but after taking a look at the history and change files I think this looks really good! LGTM

@pbsds pbsds requested a review from wegank October 13, 2023 13:14
wegank
wegank reviewed Oct 13, 2023
View reviewed changes
@soispha soispha requested a review from wegank October 16, 2023 15:53
@wegank wegank merged commit 90ec9bb into NixOS:master Oct 17, 2023
@soispha soispha mentioned this pull request Dec 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pbsds pbsds pbsds approved these changes

@wegank wegank Awaiting requested review from wegank

+1 more reviewer

@nechtarg nechtarg nechtarg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 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.

6 participants

@soispha @nechtarg @pbsds @wegank @delroth @NixOSInfra