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

Implement seek #44

Merged
merged 15 commits into from
Jan 23, 2021
Merged

Implement seek #44

merged 15 commits into from
Jan 23, 2021

Conversation

cswank
Copy link
Contributor

@cswank cswank commented Jan 21, 2021

Here we go again!

This one attempts to not break any pre-existing functionality.

cswank
cswank commented Jan 21, 2021
View reviewed changes
flac.go Outdated Show resolved Hide resolved
mewmew
mewmew reviewed Jan 22, 2021
View reviewed changes
Copy link
Member

@mewmew mewmew left a comment

Choose a reason for hiding this comment

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

A first batch of comments. Thanks a lot for making the new PR with the core functionality! It looks very clean.

flac.go Outdated Show resolved Hide resolved
flac.go Outdated Show resolved Hide resolved
flac.go Show resolved Hide resolved
flac.go Outdated Show resolved Hide resolved
flac.go Outdated Show resolved Hide resolved
flac.go Outdated Show resolved Hide resolved
flac.go Show resolved Hide resolved
flac.go Outdated Show resolved Hide resolved
@mewmew mewmew merged commit 052c3e4 into mewkiz:master Jan 23, 2021
@karlek karlek mentioned this pull request Jan 23, 2021
Closed
@jypelle
Copy link

jypelle commented Jan 23, 2021

👍

@mewmew
Copy link
Member

mewmew commented Jan 23, 2021

The PR looks good. Thanks for taking the time also to implement test cases and working through the review comments.

Really happy to finally see seek implemented in FLAC!

Cheers,

  • Henry & Robin

@jypelle jypelle mentioned this pull request Jan 23, 2021
Open
@cswank
Copy link
Contributor Author

cswank commented Jan 23, 2021 •
edited
Loading

Thanks, it was a lot of fun. I'll catch you two later.

Craig

PS. You didn't forget about this, did you Robin?

PPS. If you wouldn't mind making a release, I'd like to make a PR for beep.

@mewmew
Copy link
Member

mewmew commented Jan 23, 2021

@cswank singing as we speak!

♩♪♬♩

@mewmew mewmew mentioned this pull request Jan 23, 2021
Closed
@mewmew
Copy link
Member

mewmew commented Jan 27, 2021

PPS. If you wouldn't mind making a release, I'd like to make a PR for beep.

@cswank Now version v1.0.7 has been tagged. Would love to see seek support implemented in Beep! Looking forward to the PR :)

@cswank
Copy link
Contributor Author

cswank commented Jan 28, 2021 •
edited
Loading

PPS. If you wouldn't mind making a release, I'd like to make a PR for beep.

@cswank Now version v1.0.7 has been tagged. Would love to see seek support implemented in Beep! Looking forward to the PR :)

Already done @mewmew! We'll see where it goes: faiface/beep#111

@karlek
Copy link
Contributor

karlek commented Jan 28, 2021

Haha that's awesome! Great work @cswank 😁

This was referenced Oct 11, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mewmew mewmew mewmew left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cswank @jypelle @mewmew @karlek