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
bookstack: init at 0.31.7, nixos/bookstack: init #109711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modules do not have a version. Please remove the version from the init commit.
d8ea470
to
6f905bc
Compare
f55baf6
to
53124b5
Compare
OK, now everything should be fine. Had to fix an error with a missing description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look over this and left a few notes. I'll need to dig deeper into this.
Have you been running this in a real environment at all, or just playing around with it? I've been wanting to spend some time packaging a laravel
app for NixOS so I'm very interested in this PR.
Yes, I've been using this in my staging environment for some days now and it runs fine. The only issue is the bootstrap/cache folder being symlinked throwing errors, so I've left that out for now. |
Had to change how the config file is done to make sure the secrets are not in the nix store. |
279708f
to
472f1ea
Compare
I've changed those few things, should now be ready to merge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! What a great module! Fantastic work on this. Full points awarded ✨
I don't review packages, but as long as someone else signs off on this I suggest merge. If @etu is willing his review on the package would be quite valuable since he is familiar with composer2nix
.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
/marvin opt-in |
bump to 0.31.5 |
This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 1 package blacklisted:
1 package built:
|
Another version bump to 0.31.6 |
@ymarkus I find it terribly regrettable this hasn't been merged yet. |
Version bump to 0.31.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but didn't do an indepth review. However, it looks good on the surface level. Hoping someone more familiar with the application can also review it.
Thanks for the reviews. @jonringer I'm not sure if there are a lot of people here with knowledge of bookstack (it's quite young), but I've been using this exact module in production for more than a month now and it's working flawlessly. |
Thanks for your perseverance on this @ymarkus! Thank you 🎉 |
No worries. Thanks for your help! |
Motivation for this change
BookStack is missing as package and module.
The package was built with composer2nix.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)