nixos/ombi: add baseUrl option #284311
Closed
nixos/ombi: add baseUrl option #284311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The base URL GUI option of Ombi is known to be broken, which is very inconvenient for reverse proxying with subfolders. The suggested method is to pass the --baseurl command-line parameter. This commit thus adds an option to set this parameter to the NixOS Ombi module.
github-actions
bot
added
6.topic: nixos
NixOSInfra
added
the
12. first-time contribution
ofborg
bot
added
10.rebuild-darwin: 0
fsnkty
reviewed
Feb 6, 2024
|
I have successfully tested these changes on a server. |
fsnkty
approved these changes
Feb 18, 2024
|
Should I make these changes in this PR or open an other one?
…On February 18, 2024 1:23:16 AM GMT+01:00, nuko ***@***.***> wrote:
@nu-nu-ko approved this pull request.
changes seem fine, id be more descriptive in the enable option about what the service is though..
also seen in the module unrelated to these changes, would be nice to see `systemd.tmpfiles.settings` used insteead.
--
Reply to this email directly or view it on GitHub:
#284311 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
Id assume same PR different commit but am unsure, either way neither are obligations of yours, just random things id like to see personally. I'd wait for someone more familiar with contributing to get back to you on that one if you wish to make the changes. |
|
I'm open to making them. I just wish that what I already added be merged, since I added the new `baseUrl` option because I needed it in the first place.
…On February 20, 2024 5:42:04 AM GMT+01:00, nuko ***@***.***> wrote:
> Should I make these changes in this PR or open an other one?
Id assume same PR different commit but am unsure, either way neither are obligations of yours, just random things id like to see personally.
I'd wait for someone more familiar with contributing to get back to you on that one if you wish to make the changes.
--
Reply to this email directly or view it on GitHub:
#284311 (comment)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
wegank
added
the
12.approvals: 1
eclairevoyant
added
the
2.status: merge conflict
eclairevoyant
suggested changes
Apr 15, 2024
There was a problem hiding this comment.
Hi, thanks for the contribution! Please rebase to drop the merge commit, as well as to drop the mdDoc removal commit (this was handled treewide). (Of course, the new option shouldn't use lib.mdDoc either, as it was removed from nixpkgs.)
Otherwise the changes look good and I'll be happy to merge afterwards.
|
I had forgotten this PR, but I think it actually doesn't work. I'll have to retest it, but I think Ombi needs write access to it's HTML files, which are in the Nix store. Idk why it worked the first time.
When I have the time, I will retest this, if it works I'll make these changes. Thank you!
…On April 15, 2024 4:17:51 PM GMT+02:00, "éclairevoyant" ***@***.***> wrote:
@eclairevoyant requested changes on this pull request.
Hi, thanks for the contribution! Please rebase to drop the merge commit, as well as to drop the mdDoc removal commit (this was handled treewide).
Otherwise the changes look good and I'll be happy to merge afterwards.
--
Reply to this email directly or view it on GitHub:
#284311 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
|
Unfortunately, I don't think this is going to work. Closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The base URL GUI option of Ombi is known to be broken, which is very inconvenient for reverse proxying with subfolders. The suggested method (in Ombi's documentation) is to pass the
--baseurlcommand-line parameter.Description of changes
This commit thus adds an option to set this parameter to the NixOS Ombi module.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.