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

Hint which parser to apply first using the filename #135

Merged
merged 9 commits into from Jul 8, 2019

Conversation

julik
Copy link
Contributor

@julik julik commented Jul 6, 2019

In some situations we spend way more time applying parsers because the file is in a less-popular format - for example a WAV file. We spend quite a bit of time going through other parsers - some of which are slow. So before we have matched it as a WAV we have to walk through the JPEG parser, then through the ZIP parser (which is quite eager and slow-ish as well), and only at the end we are going to match it as a WAV file.

However, if we know the filename upfront we can do it smarter and apply the parser that is more likely to match first. Note that we won't be assuming the file is of a certain format just based on the filename - we are merely going to rearrange the order of application of the parsers, to optimise it for this particular file.

@julik julik requested a review from linkyndy July 7, 2019 00:11
@julik julik changed the title Hint which parser should be applied first using filename hinting Hint which parser should be applied first using the filename Jul 7, 2019
@julik julik changed the title Hint which parser should be applied first using the filename Hint which parser to apply first using the filename Jul 7, 2019
@@ -1,9 +1,7 @@
rvm:
- 2.2.0
- 2.3.0
- 2.4.2
- 2.5.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keeps supporting all Ruby versions that didn't reach their EOL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I wanted to do is to have sufficient "spread" of versions but run less builds. So I've set the lowest supported and the newest supported, assuming that versions in between will kinda work 🕵🏻‍♂️ Do you think it is safe enough, or would you prefer us test with all minor Ruby versions?

Copy link
Contributor

@linkyndy linkyndy Jul 8, 2019

Choose a reason for hiding this comment

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

I would do with all latest minor versions TBH:

- 2.4.6
- 2.5.5
- 2.6.3
- jruby

(I'm not up to date with jruby)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, let's do that until we bump major

lib/format_parser.rb Show resolved Hide resolved
Keep 2.2 because our major version still should
be compatible with it - this is a guarantee we give our users
@julik julik merged commit 87a3d05 into master Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants