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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

matomo: 3.14.1 -> 4.0.4 #105983

Closed
wants to merge 1 commit into from
Closed

matomo: 3.14.1 -> 4.0.4 #105983

wants to merge 1 commit into from

Conversation

Kiwi
Copy link
Member

@Kiwi Kiwi commented Dec 5, 2020

i might have a problem with compulsive formatting (especially w.r.t. lists)

there should probably be a note somewhere about a manual database upgrade step that should be done (this is the note)

Motivation for this change
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.

various versions of 4.0.x (beta versions too. currently this PR, 4.0.4) have been running on my server for a few weeks now. I tested upgrades and new installs (on... 4.0.1 i think) in a vm.

there may be a change to how it handles reverse proxies I'm not sure. also not sure if a module change is in order or not. (it doesn't like letting me log into matomo.$domain but stats.$domain is fine. it might not have liked matomo.$domain before either, I don't remember. too late for me to check now 馃槃 )

@florianjacob @aanderse

@RaghavSood
Copy link
Member

Perhaps the database update note should go into the release notes - won't save people on unstable, but I'm sure there's one or two people in the world who only use stable NixOS releases

Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

As mentioned you should add release notes. I'd like to see @florianjacob approved this before we merge it if possible.

Comment on lines +25 to +63
(mkRenamedOptionModule [
"services"
"piwik"
"enable"
] [
"services"
"matomo"
"enable"
])
(mkRenamedOptionModule [
"services"
"piwik"
"webServerUser"
] [
"services"
"matomo"
"webServerUser"
])
(mkRemovedOptionModule [
"services"
"piwik"
"phpfpmProcessManagerConfig"
]
"Use services.phpfpm.pools.<name>.settings")
(mkRemovedOptionModule [
"services"
"matomo"
"phpfpmProcessManagerConfig"
]
"Use services.phpfpm.pools.<name>.settings")
(mkRenamedOptionModule [
"services"
"piwik"
"nginx"
] [
"services"
"matomo"
"nginx"
])
Copy link
Member

Choose a reason for hiding this comment

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

I can't emphasize enough how in my opinion the formatting in this block is not an improvement 馃槙

To be clear, though, the rest of the file is fine... again, in my opinion 馃槃

Copy link
Member Author

Choose a reason for hiding this comment

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

that one specifically is what I had in mind when I made my comment about lists. 馃槃 I think nixpkgs-fmt did half of them and it was somehow worse so I did the rest so it wouldn't keep messing with it ( and that's how lists are supposed to be anyway but the reasoning probably doesn't matter here so :| ) I switched nixops-digitalocean to nixfmt since lol I found that one by accident there are a lot more in nixpkgs that it will rewrite every time. ...which seems problematic if it's meant to be put in git hooks? idk

ANYWAY I'll do something about that one and I'll see about release notes too. then I can check the documentation box for once.

@robertoschwald
Copy link

Still problem with this derivation is that some things do not work, as the directories are read-only, e.g. Geo-IP db updates or plugin installations. Imho all changeable data should reside below $dataDir. There already is a PR to fix this for GeoIP. #100617 but basically this is an upstream problem.

}
)
);
type = types.nullOr (types.submodule (recursiveUpdate
Copy link
Contributor

Choose a reason for hiding this comment

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

Those whitespace changes unnecessarily pollute the file history IMO.

@mkg20001
Copy link
Member

Matomo package is now on version 4.2.1 in master - Is this PR still relevant?

@siraben siraben closed this Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants