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

linkding: init at 1.36.0 #341963

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

linkding: init at 1.36.0 #341963

wants to merge 3 commits into from

Conversation

l0b0
Copy link
Contributor

@l0b0 l0b0 commented Sep 15, 2024

Description of changes

From the project:

Self-hosted bookmark manager that is designed be to be minimal, fast, and easy to set up using Docker.

Closes #341665.

FYI @sissbruecker

TODO: Figure out how to run django-registration tests. I tried with

  checkPhase = let
    checkPython = python.withPackages(_: nativeCheckInputs);
  in ''
    runHook preCheck

    ${lib.getExe checkPython} -m nox \
        --no-install \
        --non-interactive \
        --python=${checkPython.pythonVersion} \
        --session="tests_with_coverage(python='${checkPython.pythonVersion}', django='${lib.versions.majorMinor django.version}')"

    runHook postCheck
  '';

I also tried --no-venv, to no avail. It refuses to use the existing Python and installed packages.

I also tried

  checkPhase = let
    checkPython = python.withPackages (ps: [ps.django ps.nox]);
  in ''
      runHook preCheck

      mkdir .nox
      ${lib.getExe checkPython} -m venv --system-site-packages .nox/tests_with_coverage-python-3-12-django-4-2
      . .nox/tests_with_coverage-python-3-12-django-4-2/bin/activate
      python -m nox \
          --no-install \
          --non-interactive \
          --envdir=.nox \
          --python=${checkPython.pythonVersion} \
          --reuse-venv=always \
          --session="tests_with_coverage(python='${checkPython.pythonVersion}', django='${lib.versions.majorMinor django.version}')"

      runHook postCheck
    '';

to create and activate a virtualenv with Django and nox during the build, but despite --system-site-packages python doesn't know about the nox module.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 15, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 15, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Sep 15, 2024
@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Sep 15, 2024
@trzpiot
Copy link
Contributor

trzpiot commented Sep 16, 2024

I can take over the review for aarch64-darwin as soon as the PR is ready. 🙂

@l0b0 l0b0 force-pushed the linkding branch 3 times, most recently from 1304d4d to 714781b Compare October 9, 2024 02:53
@l0b0 l0b0 changed the title linkding: init at 1.33.0 linkding: init at 1.36.0 Oct 9, 2024
@l0b0 l0b0 marked this pull request as ready for review October 9, 2024 02:53
@l0b0 l0b0 requested a review from patka-123 October 9, 2024 02:54
@l0b0
Copy link
Contributor Author

l0b0 commented Oct 9, 2024

I can take over the review for aarch64-darwin as soon as the PR is ready. 🙂

Thanks! I can't see you in the list of possible reviewers, but feel free to look into aarch64-darwin support.

@patka-123 patka-123 removed their request for review October 11, 2024 11:36
@l0b0 l0b0 requested review from drupol, Thesola10, imincik and nh2 October 19, 2024 08:11
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

Hi!

The diff LGTM, except a few nits.

The PR must contain 2 commits ad minima:

  • 1 commit to add the package: linkding: init at 1.36.0
  • 1 commit to add the NixOS service: nixos/linkding: init (please verify in the contributor guide documentation how to format commit log messages correctly, I may be wrong here)

Since you're adding a new NixOS service, a release note entry must be added too.

pkgs/by-name/li/linkding/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/li/linkding/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/li/linkding/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/li/linkding/package.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/linkding.nix Outdated Show resolved Hide resolved
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Oct 19, 2024
@l0b0 l0b0 force-pushed the linkding branch 2 times, most recently from fdebb67 to 5bc282e Compare December 30, 2024 16:30
@l0b0 l0b0 force-pushed the linkding branch 3 times, most recently from e86e3e0 to 472bffd Compare December 30, 2024 20:51
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Jan 11, 2025
@l0b0 l0b0 requested a review from FliegendeWurst January 11, 2025 17:07
@l0b0
Copy link
Contributor Author

l0b0 commented Jan 11, 2025

Update: Got a bit further; any other tests/improvements you'd like to see?

@l0b0 l0b0 marked this pull request as ready for review January 11, 2025 17:09
@l0b0 l0b0 force-pushed the linkding branch 2 times, most recently from 9c30750 to aea19b3 Compare January 11, 2025 20:19
@nix-owners nix-owners bot requested a review from natsukium January 11, 2025 20:21
@l0b0 l0b0 force-pushed the linkding branch 2 times, most recently from 672ef76 to 2e2f052 Compare January 11, 2025 20:32
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Jan 11, 2025
Closes NixOS#341665.

Includes all relevant options
<https://linkding.link/options/#list-of-options>, leaving any options
with a `null`/`[]` default unset if not explicitly set to something
else.

Flips the semantics of some options to avoid double negatives in the
configuration.

Co-Authored-By: Pol Dellaiera <[email protected]>
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 12, 2025
@l0b0 l0b0 requested a review from drupol January 12, 2025 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 8.has: changelog 8.has: documentation This PR adds or changes documentation 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 8.has: tests This PR has tests 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: linkding
6 participants