This repository has been archived by the owner. It is now read-only.

Add python hachoir-metadata #31044

Closed
wants to merge 10 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@spk
Contributor

spk commented Jul 22, 2014

Added formula to install hachoir.

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 22, 2014

I think these should be a single formula that installs all the parts.

@spk

This comment has been minimized.

Contributor

spk commented Jul 22, 2014

Ok I think then only hachoir-metadata is needed

@adamv adamv added the new formula label Jul 23, 2014

@spk spk changed the title from Add hachoir core parser regex and metadata to Add python hachoir-metadata Jul 25, 2014

@spk

This comment has been minimized.

Contributor

spk commented Jul 25, 2014

I updated this PR to have one commit and install only hachoir-metadata

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 25, 2014

@BrewTestBot test this please

depends_on :python if MacOS.version <= :snow_leopard
resource 'hachoir-core' do

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Jul 25, 2014

Member

Use double-quotes everywhere.

end
test do
system "#{bin}/hachoir-metadata", "--version"

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Jul 25, 2014

Member

Can this test be modified to do something more substantial then e.g. --version or --help? See cmake.rb for an example of a formula with a good test. Thanks!

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 25, 2014

Looking much better! A few suggestions.

@spk

This comment has been minimized.

Contributor

spk commented Jul 25, 2014

Hope you will like it !

end
test do
output = `#{bin}/hachoir-metadata --version`

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Jul 25, 2014

Member

This is still just checking the version. Could you do something to actually run the tool on some input?

This comment has been minimized.

@spk

spk Jul 25, 2014

Contributor

Ah ok ! Yes I do that.

@spk

This comment has been minimized.

Contributor

spk commented Jul 25, 2014

I'm creating a one sized pixel png file and test the mime type is equal to image/png

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 25, 2014

Seems that brew test and brew audit are failing, I'm afraid 😭

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

It's working fine locally I'm under 10.9.4 OS X, can I have some help ?

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

It's failing under 10.7 and 10.8. Perhaps you should add a failure message to the assertions so we can see which one is failing.

spk added some commits Jul 28, 2014

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

@MikeMcQuaid It's the mime test that failing but no output... Do you have an idea ? I'm new to OS X environment.

require "formula"
class HachoirMetadata < Formula
VERSION = '1.3.3'

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Jul 28, 2014

Member

Remove this; just use version in the formula where you need to access it instead.

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

@spk I'm not sure what's going on there. We could perhaps limit it to Mavericks and add a comment if someone else wants to fix it in future? @mistydemeo @adamv @jacknagel Any thoughts?

spk added some commits Jul 28, 2014

hachoir: depends on mavericks
no outputs for mime type on 10.7 and 10.8
@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

If people don't want to debug my stuff I can understand. I limit then to Mavericks. Thanks for the helps !

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

In fact, I've got an idea of what it might be. Those versions are running Ruby 1.8 so the unicode output may be different. I think it might be better to just create a PNG and put it in the Homebrew repo. @jacknagel @mistydemeo @adamv any objections to me putting a single, small image in the repo to use as test data? It would also help with formulae like imagemagick.

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

Ah ok cool, where can I put it ?

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

@spk Hold off for a bit while I wait for the other maintainers to weigh in.

@adamv

This comment has been minimized.

Contributor

adamv commented Jul 28, 2014

A small png in the test folder seems fine.

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

@MikeMcQuaid Sorry ! I'm waiting for GO !

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

@spk Can you put a 1x1 PNG file in Library/Homebrew/test/fixtures/test.png and modify the test to use that? Thanks!

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

Roger o>

@spk

This comment has been minimized.

Contributor

spk commented Jul 28, 2014

\o/

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jul 28, 2014

Thanks for all your work here!

@spk spk deleted the spk:add_python_hachoir branch Jul 29, 2014

aerickson added a commit to aerickson/homebrew that referenced this pull request Aug 3, 2014

@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016

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