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

Change the low_latency option to a flag #1228

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

vladimir-kazakov
Copy link
Contributor

This PR fixes the issue #1224. It changes the low_latency option that takes a boolean value to a flag, which can be used without arguments. If the flag is present, the low latency mode is enabled; otherwise, it's disabled.

Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

I like the change

@coveralls
Copy link
Collaborator

Coverage Status

Coverage remained the same at 82.605% when pulling 73621ed on vladimir-kazakov:low-latency-as-flag into d7599d6 on xiph:master.

@tdaede
Copy link
Collaborator

tdaede commented Apr 22, 2019

Does it now reject --low_latency true/false? If so I'll need to adjust the AWCY arguments to match.

Copy link
Collaborator

@rom1v rom1v left a comment

Choose a reason for hiding this comment

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

Thank you 👍

This is a breaking change from the user point of view, maybe we should increase rav1e version to 0.2.0?

@tdaede
Copy link
Collaborator

tdaede commented Apr 22, 2019

We don't have a versioning policy yet, so I just made one: #1231

@vladimir-kazakov
Copy link
Contributor Author

Does it now reject --low_latency true/false?

@tdaede, yes, using any value with a flag leads to an error, because flags don't expect values.

This is a breaking change from the user point of view, maybe we should increase rav1e version to 0.2.0?

I guess the version should change for a specific release, not because of every PR with a breaking change. In other words, it should be okay to have breaking changes in the master (development) branch, but the version number should be increased before making a release.

@rom1v
Copy link
Collaborator

rom1v commented Apr 23, 2019

I guess the version should change for a specific release, not because of every PR with a breaking change. In other words, it should be okay to have breaking changes in the master (development) branch, but the version number should be increased before making a release.

Yes, absolutely.

@lu-zero
Copy link
Collaborator

lu-zero commented Apr 23, 2019

I guess we could put a label so when we cut releases we won't forget.

@tdaede tdaede merged commit ae5a21e into xiph:master Apr 24, 2019
@vladimir-kazakov vladimir-kazakov deleted the low-latency-as-flag branch April 25, 2019 05:41
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.

5 participants