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

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

posting: init at 2.3.0 #353386

wants to merge 3 commits into from

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

Pretty much ready to merge I think. I need to update the PR to target the latest master to update to the latest posting version (2.3.0). I'd appreciate if someone could tell me or point to a resource on how to properly do the rebase/merge, since I accidentally mass pinged dozens of people last time I tried.

@taha-yassine
Copy link
Contributor

I'd appreciate if someone could tell me or point to a resource on how to properly do the rebase/merge, since I accidentally mass pinged dozens of people last time I tried.

From a comment in the original PR:

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.

Also, the commit that triggered the mass pinging changed a lot of code outside of the PR's scope which is why that happened. If you rebase to catch up with master which you're targeting, you should be fine. Just double check before pushing to make sure you're not accidentally modifying code you don't want to :)

@jorikvanveen jorikvanveen marked this pull request as draft January 6, 2025 18:21
@jorikvanveen jorikvanveen force-pushed the init-posting-ihategit branch from 0d9a0f3 to 3490d29 Compare January 6, 2025 18:21
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 6, 2025
@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!

pkgs/development/python-modules/textual-autocomplete/default.nix
version = "3.0.0a13";
pyproject = true;

# Alpha versions of this packages are only available on Pypi for some reason
Copy link
Contributor

Choose a reason for hiding this comment

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

The current latest alpha source code is available on the version-two branch

pkgs/by-name/po/posting/package.nix

nativeBuildInputs = [ python3Packages.hatchling ];

dependencies =
Copy link
Contributor

Choose a reason for hiding this comment

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

Would make sense to put a with python3Packages; here

pkgs/by-name/po/posting/package.nix
src = fetchFromGitHub {
owner = "darrenburns";
repo = "posting";
rev = "refs/tags/${version}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably best to just use tag = version;

pkgs/by-name/po/posting/package.nix
fetchFromGitHub,
python3Packages,
}:
let
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty let in?

pkgs/by-name/po/posting/package.nix
python3Packages.textual
]
++ python3Packages.httpx.optional-dependencies.brotli
++ python3Packages.textual.optional-dependencies.syntax;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably include xorg.xdb so X resources themes can work

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

@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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Package request: posting
6 participants
@jorikvanveen @xanderio @taha-yassine @Aleksanaa @justDeeevin @wegank