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

posting: init at 2.0.1 #352663

Closed
wants to merge 3 commits into from
Closed

Conversation

jorikvanveen
Copy link

@jorikvanveen jorikvanveen commented Oct 31, 2024

This adds the posting HTTP client. It's like insomnia or postman but it's a TUI. It also adds me to the maintainer list.
https://github.com/darrenburns/posting
https://posting.sh/

Fixes #351936

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.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Oct 31, 2024
@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Oct 31, 2024
pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
@ryand56
Copy link
Member

ryand56 commented Nov 1, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 352663


x86_64-linux

✅ 2 packages built:
  • posting
  • posting.dist

@jorikvanveen
Copy link
Author

Thanks for reviewing!

pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
# We patch the pyproject.toml to allow a slightly outdated watchfiles dependency.
# The correct version is not in nixpkgs yet and overriding the version is not trivial.
# (i tried)
patches = [ ./allow-watchfiles-v22.patch ];
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to patch all the other things lower as well, or would that fail?

Copy link
Author

Choose a reason for hiding this comment

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

Most requirements could probably be patched to the current nixpkgs version. Is this preferred over making sure the standard pyproject.toml is satisfied?

Copy link
Member

Choose a reason for hiding this comment

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

Its case by case to be completely honest. I think just due to the sheer amount of overriding youre doing (which is a maintenance hassle) itd be generally easier for you and others if it was compatible with nixpkgs as-is. Of course, if its not possible thats a different problem.

Copy link
Member

@Frontear Frontear Nov 1, 2024

Choose a reason for hiding this comment

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

From matrix: https://matrix.to/#/!kjdutkOsheZdjqYmqp%3Anixos.org/%241sPNgtk0FyKTPgCoj5JHYfRZe4ZJ5Fq6d-zsNhRKoYM

Since this is an app it should be fine to use overrides here, though instead use packageOverrides (idk how this works but I can take a look at it later if you want)

Copy link
Author

Choose a reason for hiding this comment

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

Alright I've made the pyproject use the nixpkgs version for everything except pydantic-settings and textual. The former causes a crash when I use the nixpkgs version and the latter is explicitly pinned by the maintainer of posting so I assume it will cause breakages if I relax that.

pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
Comment on lines 11 to 71
# Current nixpkgs version is too outdated
pydantic-settings = (
py-pkgs.pydantic-settings.overrideAttrs (oldAttrs: rec {
version = "2.6.0";
src = fetchFromGitHub {
owner = "pydantic";
repo = "pydantic-settings";
rev = "refs/tags/v${version}";
hash = "sha256-gJThzYJg6OIkfmfi/4MVINsrvmg+Z+0xMhdlCj7Fn+w=";
};
propagatedBuildInputs = [
pydantic
py-pkgs.python-dotenv
];
})
);

# Current nixpkgs version is too outdated
httpx = (
py-pkgs.httpx.overrideAttrs (oldAttrs: rec {
version = "0.27.2";
src = fetchFromGitHub {
owner = "encode";
repo = oldAttrs.pname;
rev = "refs/tags/${version}";
hash = "sha256-N0ztVA/KMui9kKIovmOfNTwwrdvSimmNkSvvC+3gpck=";
};
nativeBuildInputs = oldAttrs.nativeBuildInputs ++ [ py-pkgs.zstandard ];
})
);

# Current nixpkgs version is too outdated
textual = (
py-pkgs.textual.overrideAttrs (oldAttrs: rec {
version = "0.85.0";
src = fetchFromGitHub {
owner = "Textualize";
repo = "textual";
rev = "refs/tags/v${version}";
hash = "sha256-ROq/Pjq6XRgi9iqMlCzpLmgzJzLl21MI7148cOxHS3o=";
};
})
);

# Current nixpkgs version is too outdated
pydantic = (
py-pkgs.pydantic.overrideAttrs (oldAttrs: rec {
version = "2.9.2";
src = fetchFromGitHub {
owner = "pydantic";
repo = "pydantic";
rev = "refs/tags/v${version}";
hash = "sha256-Eb/9k9bNizRyGhjbW/LAE/2R0Ino4DIRDy5ZrQuzJ7o=";
};
propagatedBuildInputs = [
pydantic-core
py-pkgs.annotated-types
py-pkgs.jsonschema
];
})
);
Copy link
Member

Choose a reason for hiding this comment

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

Can these be updated? (honestly this is probably non-trivial so if it is you can disregard this comment)

pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/po/posting/package.nix Outdated Show resolved Hide resolved
@jorikvanveen
Copy link
Author

So apparently #325971 exists, that's embarrassing... It looks like it has gone somewhat stale though, should I close this?

@Frontear
Copy link
Member

Frontear commented Nov 2, 2024

So apparently #325971 exists, that's embarrassing... It looks like it has gone somewhat stale though, should I close this?

@taha-yassine are you still willing to complete your PR? If not, this PR is doing the same thing.

@taha-yassine
Copy link
Contributor

So apparently #325971 exists, that's embarrassing... It looks like it has gone somewhat stale though, should I close this?

@taha-yassine are you still willing to complete your PR? If not, this PR is doing the same thing.

I haven't had time to finish working on my PR since I opened it so I'm glad you got posting to work with Nix. I will close my PR in favor of this one.

@taha-yassine taha-yassine mentioned this pull request Nov 2, 2024
13 tasks
@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/` 6.topic: vim 6.topic: xfce The Xfce Desktop Environment labels Nov 2, 2024
@khaneliman
Copy link
Contributor

khaneliman commented Nov 2, 2024

Intended to be this many commits? Looks like a lot of unrelated stuff.

@jorikvanveen
Copy link
Author

jorikvanveen commented Nov 2, 2024

No, not intentional. It appears something has gone wrong when I tried to rebase this branch to the latest master

Sorry for all the pings!

@github-actions github-actions bot removed 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/` 6.topic: vim 6.topic: xfce The Xfce Desktop Environment 6.topic: nodejs 6.topic: vscode labels Nov 2, 2024
@SigmaSquadron
Copy link
Contributor

SigmaSquadron commented Nov 2, 2024

It looks like you accidentally mass-pinged a bunch of people, which are now subscribed and getting notifications for everything in this pull request. Unfortunately, they cannot be automatically unsubscribed from the issue (removing review request does not unsubscribe), therefore development cannot continue in this pull request anymore.

Please create a new pull request, link back to this one and ping the people actually involved in here over there. For the next time, remember to set your PR to draft status before rebasing. In draft status, you can preview the list of maintainers that are about to be requested for review, which allows you to sidestep this issue. Setting your pull request to draft prior to rebasing is strongly recommended.

This is not a bulletproof method, though, as OfBorg still does review requests even on draft PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: posting
7 participants