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

matomo: refactor, matomo-beta: remove #374022

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

Conversation

niklaskorz
Copy link
Contributor

@niklaskorz niklaskorz commented Jan 15, 2025

  • matomo-beta: removed as it mostly just pointed to the latest stable version and a specific beta or release-candidate can now be easily obtained by using matomo.overrideAttrs, e.g.:
    pkgs.matomo.overrideAttrs (finalAttrs: prevAttrs: {
      version = "5.2.0-b5";
      src = prevAttrs.src.override {
        hash = "sha256-jMm9BvncW0PIbddDuped07kRNkfXBzFhyka0hWACCuo=";
      };
    })
  • matomo:
    • moved package and test definition to the top-level of their file as we now only have one version of matomo left inside nixpkgs
    • moved to pkgs/by-name
    • made the package nix-update compatible so we never miss security updates for multiple months again (as was the case with 5.1.2: matomo_5: 5.1.1 -> 5.1.2 #363621)
    • added myself as maintainer

Once this is merged, I will make a follow-up PR on release-24.11 to make automatic backports possible again, as the last few updates had to be manually backported due to the rename of matomo_5 to matomo.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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: documentation This PR adds or changes documentation 8.has: changelog labels Jan 15, 2025
@niklaskorz niklaskorz force-pushed the matomo-refactor branch 4 times, most recently from 726b78d to 36d489d Compare January 15, 2025 11:41
@osnyx
Copy link
Contributor

osnyx commented Jan 15, 2025

The large test changes might clash with #373894, let's see how this goes.

Copy link
Contributor

@osnyx osnyx left a comment

Choose a reason for hiding this comment

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

Code LGTM, still waiting for the aarch64 CI runs.

@niklaskorz
Copy link
Contributor Author

niklaskorz commented Jan 15, 2025

The large test changes might clash with #373894, let's see how this goes.

If you prefer I can undo most test-related changes and only apply the removal of matomo-beta to the tests.

@osnyx
Copy link
Contributor

osnyx commented Jan 15, 2025

@niklaskorz Let's keep your test-related changes, the test diff for #373894 is pretty small. So if we commit to merging this PR first, I can rebase and adjust my PR.

@osnyx
Copy link
Contributor

osnyx commented Jan 15, 2025

Although, now that I think of it, #373894 might be a bit more urgent and it's important that it can be backported to 24.11. Because the matomo-5.2.x there currently is affected by the issue fixed there.

@niklaskorz
Copy link
Contributor Author

Although, now that I think of it, #373894 might be a bit more urgent and it's important that it can be backported to 24.11. Because the matomo-5.2.x there currently is affected by the issue fixed there.

In that case feel free to merge that first and I can rebase this PR afterwards. I'll also review your PR in a moment.

@niklaskorz
Copy link
Contributor Author

Rebased and resolved conflicts with changes from #373894

nixos/tests/matomo.nix Outdated Show resolved Hide resolved
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 16, 2025
Copy link
Member

@leona-ya leona-ya left a comment

Choose a reason for hiding this comment

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

Thanks!

@niklaskorz niklaskorz added 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jan 17, 2025
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 8.has: changelog 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants