etebase-server: 0.13.1 -> 0.14.2

[phaer]

Description of changes

The automated update in #318528 failed, as that still contains a python interpreter overridden to use pydantic_1. Upstream has upgraded to pydantic 2 and doesn't work with pydantic_1 anymore, so we drop the override and do the upgrade, which lets the tests pass.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[felschr]

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

2 packages built:

• etebase-server
• etebase-server.dist

[SuperSandro2000]

If it only works with a newer pydantic, it should be done in the same commit.

[phaer]

If it only works with a newer pydantic, it should be done in the same commit.

It's two different logical changes, but I hereby grant the person merging this permission to squash as they see fit. :)

If you believe that it's mandatory, @SuperSandro2000: Please cite & link the relevant contribution guidelines or clarify that this is just a personal opinion. Thank you!

[felschr]

I think @SuperSandro2000 probably has this rule from the Commit conventions in mind:

• If you have commits pkg-name: oh, forgot to insert whitespace: squash commits in this case. Use git rebase -i. See Squashing Commits for additional information.

Admittedly it is not 100 % clear that it applies here.
I think one of the general ideas is that all commits should build. In our case updating the package without changing the dependency creates a broken build. This could lead to unexpected behavior, e.g. when bisecting to find a different issue later.

[SuperSandro2000]

Commits should generally be done in a way that each of them works and compiles (as already written above). Sometimes it is not avoidable or only doable with extra work, then I personally think it is fine to skip that. Why is that desirable? For example when git bisecting it can throw you off if a commit doesn't work and make your debugging harder than necessary.
Especially if you need to change dependencies that should always be done in the update commit but e.g. updates to dependencies should be done before.

It's two different logical changes

No, it isn't. For the update to work we must unpin pydantic. If we would just merge the update commit without the unpin it would be rightfully wrong and a good candida for immediate revert. We also can't separate the changes or reorder them.

Please cite & link the relevant contribution guidelines or clarify that this is just a personal opinion.

That is common git knowledge on how to write a good history.
Also the documentation writes down broad rules that apply everywhere. It cannot be so specific that it covers every case that can come up in review. In question we find a consensus in the specific case and see what fits best and the doc looses.

[phaer]

Thank both of you for your in-depth replies, I am trying to keep it short as my whole goal is to avoid discussions like these and get them into linter or ci checks where possible and rules into writing where necessary.

I always agreed that this can be squashed on merge. I personally find it easier during reviews if each logical change is in a separate commit, what matters IMO is the merged history.

I think one of the general ideas is that all commits should build.

I understand that's an idea some of us hold but afaik that is neither true for present-day nixpkgs nor is it - technically - in the contribution guidelines (or is it? I'd love to learn :)).

Personally I think it's important that each commit that is evaluated by hydra builds, not necessarily each commit. But if we need a rule for that (I think we do) we should either make it more easy to find or start an RFC about and ideally some tooling around it.

And while we are at the topic of RFCs: I believe #336200 comes up with a good definition of nitpicking and this thread might be at least close to match :)