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.

Sign up for GitHub

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

jiten: init at 1.0.0 #110900

Merged
merged 2 commits into from Apr 19, 2021
Merged

jiten: init at 1.0.0 #110900

merged 2 commits into from Apr 19, 2021

Conversation

obfusk
Copy link
Member

@obfusk obfusk commented Jan 26, 2021 •
edited

Motivation for this change

I'm the developer of Jiten Japanese Dictionary. A NixOS user wanted to install it, so I made a nix expression. Since having it in nixpkgs would be even better, I'm making this PR :)

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.

SuperSandro2000
SuperSandro2000 reviewed Jan 27, 2021
View reviewed changes
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

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).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

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

1 package built:
  • jiten

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

jiten:

Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
You have the following options to fix this:

  • Tell pytest(CheckHook) where to find the tests included in the package
  • Check if the GitHub Repo contains tests but they are not shipped with Pypi. If so please switch to fetchFromGitHub.
  • If the Packages does not contain any tests add 'doCheck = false;' and a pythonImportsCheck.

@sikmir
Copy link
Member

sikmir commented Feb 5, 2021 

result/bin/jiten gui
Traceback (most recent call last):
  File "/nix/store/wkw6fsjasr7jbbrlakxxpbiapa8hws42-python3-3.8.7/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/nix/store/wkw6fsjasr7jbbrlakxxpbiapa8hws42-python3-3.8.7/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/nix/store/dzpp7zxzwlzpamldms8w5h4vnwlm8i69-jiten-0.4.0/lib/python3.8/site-packages/jiten/cli.py", line 564, in <module>
    cli(prog_name = name)
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/nix/store/aph3vafycfbj9325vgcm7y1zlx5v88j6-python3.8-click-7.1.2/lib/python3.8/site-packages/click/decorators.py", line 21, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/nix/store/dzpp7zxzwlzpamldms8w5h4vnwlm8i69-jiten-0.4.0/lib/python3.8/site-packages/jiten/cli.py", line 546, in gui
    start(link)
  File "/nix/store/dzpp7zxzwlzpamldms8w5h4vnwlm8i69-jiten-0.4.0/lib/python3.8/site-packages/jiten/gui.py", line 34, in start
    import webview
ModuleNotFoundError: No module named 'webview'

@obfusk obfusk changed the title jiten: init at 0.4.0 jiten: init at 1.0.0 Feb 20, 2021
@obfusk
Copy link
Member Author

obfusk commented Feb 20, 2021

ModuleNotFoundError: No module named 'webview'

Because the gui is optional and depends on pywebview I've removed the jiten-gui script from the package and improved the error message a bit.

@obfusk
Copy link
Member Author

obfusk commented Feb 20, 2021

@SuperSandro2000 @sikmir thanks for the feedback. I've just released v1.0.0 and updated the PR accordingly.

@obfusk obfusk mentioned this pull request Feb 20, 2021
Closed
@obfusk
Copy link
Member Author

obfusk commented Apr 15, 2021

I resolved the merge conflict. Is there anything else preventing this from being merged?

SuperSandro2000
SuperSandro2000 requested changes Apr 16, 2021
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please remove the merge commit. I cannot squash this PR due to the maintainer commit and it makes the history hard to follow when merged.

pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Show resolved Hide resolved
pkgs/applications/misc/jiten/default.nix Outdated Show resolved Hide resolved
@obfusk obfusk marked this pull request as draft April 17, 2021 10:39
obfusk and others added 2 commits April 17, 2021 12:46
@obfusk
maintainers: add obfusk
e69f9db
@obfusk @SuperSandro2000
jiten: init at 1.0.0
e2066ea 
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@obfusk
Copy link
Member Author

obfusk commented Apr 17, 2021 •
edited

Please remove the merge commit. I cannot squash this PR due to the maintainer commit and it makes the history hard to follow when merged.

Sorry. I knew I should have just done a command-line rebase instead of using the GitHub interface 😅

@obfusk obfusk marked this pull request as ready for review April 17, 2021 11:07
@SuperSandro2000
Copy link
Member

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).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

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

1 package built:
  • jiten

@SuperSandro2000 SuperSandro2000 merged commit 6734491 into NixOS:master Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sikmir sikmir sikmir left review comments

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@obfusk @SuperSandro2000 @sikmir @Janik-Haag