Skip to content
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

Jump to bottom

filebot: init at 4.9.1 #97298

Merged
merged 2 commits into from Sep 16, 2020
Merged

filebot: init at 4.9.1 #97298

merged 2 commits into from Sep 16, 2020

Conversation

felschr
Copy link
Member

@felschr felschr commented Sep 6, 2020

Motivation for this change

This continues the work from #67849, implementing some of the suggested changes and updating to the most recent version.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@felschr
Copy link
Member Author

felschr commented Sep 6, 2020

FileBot is starting and it seems to be working fine, though I wasn't able to test the actual renaming feature because I lost my FileBot license key :/

@ofborg ofborg bot requested a review from gleber September 6, 2020 23:34
drewrisinger
drewrisinger suggested changes Sep 8, 2020
View reviewed changes
Copy link
Contributor

@drewrisinger drewrisinger left a comment •
edited

Choose a reason for hiding this comment

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

  • Diff could use a few small improvements
  • Maintainer commit needs broken out separately
  • Build works as tested so far with nix-review:
https://github.com/NixOS/nixpkgs/pull/97298
1 package built:
filebot

I do get the following warning when I run filebot -help (on Ubuntu via WSL)

/nix/store/...-filebot-4.9.1/opt/.filebot.sh-wrapped: line 36: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)

maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Show resolved Hide resolved
pkgs/applications/video/filebot/default.nix Outdated Show resolved Hide resolved
@felschr felschr force-pushed the feat/filebot branch 2 times, most recently from 5a0e761 to d97fbfa Compare September 8, 2020 22:10
@felschr
Copy link
Member Author

felschr commented Sep 8, 2020 •
edited

I don't get the setlocale warning on my NixOS setup.
It comes from the original filebot.sh where it does the following:

# force JVM language and encoding settings
export LANG="en_US.UTF-8"
export LC_ALL="en_US.UTF-8"

I don't think we should touch that.

@ofborg ofborg bot requested a review from gleber September 8, 2020 23:02
drewrisinger
drewrisinger approved these changes Sep 9, 2020
View reviewed changes
Copy link
Contributor

@drewrisinger drewrisinger left a comment

Choose a reason for hiding this comment

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

  • Diff LGTM
  • Commits LGTM
  • Builds via nix-review:
https://github.com/NixOS/nixpkgs/pull/97298
1 package built:
filebot

[nix-shell:~/.cache/nixpkgs-review/pr-97298-1]$ filebot -version
/nix/store/38a8m235x6q0qhcjrl65xvgmw6icwa88-filebot-4.9.1/opt/.filebot.sh-wrapped: line 36: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
FileBot 4.9.1 (r7372) / OpenJDK Runtime Environment 11.0.8 / Linux 4.19.104-microsoft-standard (amd64)

@felschr
Copy link
Member Author

felschr commented Sep 14, 2020

Should I somehow retrigger the EditorConfig GitHub action for this to get merged?

@drewrisinger
Copy link
Contributor

@felschr I think maintainers (people w/ commit access) just haven't seen this thread. Maybe mention it in the NixOS discourse thread: https://discourse.nixos.org/t/prs-already-reviewed/2617/220

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/227

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/228

@drewrisinger
Copy link
Contributor

@felschr, try rebasing this on master & seeing if the EditorConfig error disappears. Easier than trying to explain.

git checkout master
git pull upstream master 
git checkout feat/filebot 
git rebase -i master 
git push -f

@AndersonTorres AndersonTorres merged commit 8a715e2 into NixOS:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drewrisinger drewrisinger drewrisinger approved these changes

@gleber gleber Awaiting requested review from gleber

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@felschr @drewrisinger @nixos-discourse @gleber @AndersonTorres