Skip to content

ocrfeeder: Add standard-imghdr to ocrfeeder dependencies#480779

Closed
2hexed wants to merge 2 commits intoNixOS:masterfrom
2hexed:patch-11
Closed

ocrfeeder: Add standard-imghdr to ocrfeeder dependencies#480779
2hexed wants to merge 2 commits intoNixOS:masterfrom
2hexed:patch-11

Conversation

@2hexed
Copy link
Member

@2hexed 2hexed commented Jan 16, 2026

This seems to be the required dependency. Without it the program fails to open and errors out with imghdr not found.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@2hexed 2hexed requested a review from Bot-wxt1221 January 16, 2026 18:40
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jan 16, 2026
@2hexed 2hexed marked this pull request as ready for review January 16, 2026 18:43
@nixpkgs-ci nixpkgs-ci bot requested a review from doronbehar January 16, 2026 18:57
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 18, 2026
]
))
];

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this unneeded newline.

Copy link
Member Author

@2hexed 2hexed Jan 18, 2026

Choose a reason for hiding this comment

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

The newline is nice to have for readability, unsure why it wasn't there in the first place

Copy link
Contributor

Choose a reason for hiding this comment

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

The newline is nice to have for readability, unsure why it wasn't there in the first place

It is arguable, and even if this was very convincing, it would have been more correct to make readability changes in a separate commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, I'll make another commit for this!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, I'll make another commit for this!

I really prefer you won't :) unless you will make substantial other readability improvements. TBH I don't understand why people care all that much about newlines. If you were more involved with this expression in general I wouldn't have mind, but otherwise I prefer not 🙏 .

Copy link
Member Author

Choose a reason for hiding this comment

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

its alright its just a single newline insertion, i don't think it'll matter much anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems pretty insane to insist on that new line, especially if you agree with making a separate commit for that. Since (noticed only now) the commits also have the bad prefix - nixpkgs/ocrfeeder instead of ocrfeeder, I will merge the key change suggested here elsewhere:

Copy link
Member Author

@2hexed 2hexed Jan 18, 2026

Choose a reason for hiding this comment

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

its really not insane and the prefix is also acceptable, if anything, it only helps understanding where we're merging the PR, you may have a look at other PRs if you're unsure

Copy link
Contributor

Choose a reason for hiding this comment

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

its really not insane and the prefix is also acceptable, if anything, it only helps understanding where we're merging the PR, you may have a look at other PRs if you're unsure

As for the commits separation, newlines and readability, Nixpkgs doesn't have concrete rules written unfortunately, but that is a WIP - see #400934 . As for the commit prefix, there is no debate, see #431688 .

This seems to be the required dependency. Without it the program fails to open and errors out with imghdr not found.
@doronbehar doronbehar changed the title nixpkgs/ocrfeeder: Add standard-imghdr to ocrfeeder dependencies ocrfeeder: Add standard-imghdr to ocrfeeder dependencies Jan 18, 2026
@doronbehar doronbehar closed this Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants