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.

Sign up for GitHub

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

Jump to bottom

posting: init at 2.3.0 #353386

Merged
merged 3 commits into from
Feb 14, 2025
Merged

posting: init at 2.3.0 #353386

merged 3 commits into from
Feb 14, 2025

Conversation

jorikvanveen
Copy link

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/

I was previously working on this in #352663, until I did something incredibly cursed while rebasing, causing me to circumvent the anti mass-ping mechanisms and ping 10+ maintainers.

People involved with the original PR:
@ryand56
@Frontear

Fixes #351936, #327855

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: python 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Nov 3, 2024
@nix-owners nix-owners bot requested a review from natsukium November 3, 2024 14:08
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch 2 times, most recently from f6454b5 to eb93125 Compare November 3, 2024 14:16
@xanderio
Copy link
Contributor

xanderio commented Nov 3, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353386

aarch64-darwin

❌ 2 packages failed to build:
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist
✅ 2 packages built:
  • posting
  • posting.dist

@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-linux: 1-10 labels Nov 4, 2024
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from eb93125 to b2c04a7 Compare November 4, 2024 11:51
@jorikvanveen
Copy link
Author

Thanks for reviewing @xanderio. That build failure should be fixed now!

@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch 2 times, most recently from c9ffc98 to cb2ecd1 Compare November 4, 2024 13:26
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch 2 times, most recently from 39200c3 to 5b5b4b1 Compare November 9, 2024 12:42
@jorikvanveen
Copy link
Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353386

x86_64-linux

❌ 2 packages failed to build:
  • isponsorblocktv
  • isponsorblocktv.dist
✅ 59 packages built:
  • browsr
  • browsr.dist
  • frogmouth
  • frogmouth.dist
  • harlequin
  • harlequin.dist
  • memray
  • memray.dist
  • oterm
  • oterm.dist
  • posting
  • posting.dist
  • python311Packages.manifestoo
  • python311Packages.manifestoo.dist
  • python311Packages.pytest-textual-snapshot
  • python311Packages.pytest-textual-snapshot.dist
  • python311Packages.textual
  • python311Packages.textual-dev
  • python311Packages.textual-dev.dist
  • python311Packages.textual-fastdatatable
  • python311Packages.textual-fastdatatable.dist
  • python311Packages.textual-slider
  • python311Packages.textual-slider.dist
  • python311Packages.textual-textarea
  • python311Packages.textual-textarea.dist
  • python311Packages.textual-universal-directorytree
  • python311Packages.textual-universal-directorytree.dist
  • python311Packages.textual.dist
  • python312Packages.manifestoo
  • python312Packages.manifestoo.dist
  • python312Packages.pytest-textual-snapshot
  • python312Packages.pytest-textual-snapshot.dist
  • python312Packages.textual
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist
  • python312Packages.textual-dev
  • python312Packages.textual-dev.dist
  • python312Packages.textual-fastdatatable
  • python312Packages.textual-fastdatatable.dist
  • python312Packages.textual-slider
  • python312Packages.textual-slider.dist
  • python312Packages.textual-textarea
  • python312Packages.textual-textarea.dist
  • python312Packages.textual-universal-directorytree
  • python312Packages.textual-universal-directorytree.dist
  • python312Packages.textual.dist
  • rich-cli
  • rich-cli.dist
  • smassh
  • smassh.dist
  • streamcontroller
  • tftui
  • tftui.dist
  • toolong
  • toolong.dist
  • upiano
  • upiano.dist
  • wsrepl
  • wsrepl.dist

Sponsor block fails with this error, seems unrelated to these changes though.

> Checking runtime dependencies for isponsorblocktv-2.2.1-py3-none-any.whl
>   - rich==13.7.1 not satisfied by version 13.8.1

@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch 2 times, most recently from 0eb2605 to 0d9a0f3 Compare November 13, 2024 15:00
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 10, 2024
@taha-yassine
Copy link
Contributor

Any updates on this?

@jorikvanveen
Copy link
Author

jorikvanveen commented Jan 6, 2025 •
edited
Loading

Okay, no mass ping this time 😅, but somehow I still included one commit which isn't mine. Sorry about that! To be honest I've never used the git rebase command before contributing here so there are still some misunderstandings. I guess this happened because at some point I did a rebase -i on the commit before the one that my branch was based on.

I checked git diff upstream/master before pushing and only my changes were visible, how come this other commit shows up in the PR anyway?

EDIT, note to self:
Okay, I fixed it with git rebase upstream/master and learned you can preview which commits will show up in the PR using git log upstream/master..(your branch)

@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from 3490d29 to 11f5fbd Compare January 6, 2025 18:45
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from 11f5fbd to 1351099 Compare January 6, 2025 18:55
@jorikvanveen jorikvanveen marked this pull request as ready for review January 6, 2025 18:58
@jorikvanveen
Copy link
Author

Alright, as far as I'm concerned this is ready to be reviewed and/or merged now!

@jorikvanveen jorikvanveen changed the title posting: init at 2.0.1 posting: init at 2.3.0 Jan 8, 2025
justDeeevin
justDeeevin suggested changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@justDeeevin justDeeevin left a comment

Choose a reason for hiding this comment

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

Good work!

@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from 1351099 to 0e31014 Compare January 22, 2025 11:22
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from 0e31014 to 5727cac Compare January 22, 2025 11:23
@taha-yassine
Copy link
Contributor

If this PR is ready can it be merged?
@Aleksanaa

GaetanLepage
GaetanLepage reviewed Feb 9, 2025
View reviewed changes
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Feb 10, 2025
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353386

x86_64-linux

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

aarch64-linux

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

x86_64-darwin

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

aarch64-darwin

❌ 4 packages failed to build:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

@GaetanLepage
Copy link
Contributor

textual is failing on aarch64-darwin. I address it in #381825.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353386

x86_64-linux

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

aarch64-linux

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

x86_64-darwin

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

aarch64-darwin

✅ 4 packages built:
  • posting
  • posting.dist
  • python312Packages.textual-autocomplete
  • python312Packages.textual-autocomplete.dist

GaetanLepage
GaetanLepage approved these changes Feb 14, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

All good now, thanks!

@GaetanLepage GaetanLepage merged commit 4bf98eb into NixOS:master Feb 14, 2025
29 of 30 checks passed
@teto teto mentioned this pull request Feb 14, 2025
Closed
@mweinelt
Copy link
Member

mweinelt commented Mar 10, 2025 •
edited
Loading

All good now, thanks!

This package references python3, the default interpreter version from within the packageset. It should not have been merged like that.

@jorikvanveen
Copy link
Author

Whoops, I'm not sure I completely understand. All the existing packages I can find that are under pkgs/by-name and use buildPythonApplication seem to reference python.pkgs, python3.pkgs or python3xxPackages. Are you saying I'm supposed to be using the first one? Can you give an example of a package that handles this correctly?

@mweinelt
Copy link
Member

mweinelt commented Mar 10, 2025 •
edited
Loading

This is regarding textual-autocomplete, not posting.

When referencing python3.pkgs.buildPythonPackage you pin the package to the default python version we ship. But the python package set must work for multiple python versions, so you must use buildPythonPackge directtly.

@jorikvanveen jorikvanveen mentioned this pull request Mar 11, 2025
Closed
13 tasks
@GaetanLepage
Copy link
Contributor

GaetanLepage commented Mar 11, 2025 •
edited
Loading

This is regarding textual-autocomplete, not posting.

When referencing python3.pkgs.buildPythonPackage you pin the package to the default python version we ship. But the python package set must work for multiple python versions, so you must use buildPythonPackge directtly.

My bad, I know about this and definitely ask people to change those. Here I completely missed it.

I have opened #388904, but maybe we'll integrate these changes to #382551.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aleksanaa Aleksanaa Aleksanaa left review comments

@justDeeevin justDeeevin justDeeevin requested changes

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@ethancedwards8 ethancedwards8 ethancedwards8 approved these changes

@natsukium natsukium Awaiting requested review from natsukium

Assignees
No one assigned
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-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Package request: posting
9 participants
@jorikvanveen @xanderio @taha-yassine @GaetanLepage @mweinelt @Aleksanaa @ethancedwards8 @justDeeevin @wegank