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

Cleanup, updates and made more generic #2

Merged
merged 5 commits into from
Mar 7, 2021
Merged

Conversation

sneljo1
Copy link
Contributor

@sneljo1 sneljo1 commented Feb 8, 2021

This is an improvement to the original module to make it more extensible for other users like myself. I think this is a good basis to build upon further if needed.

Instead of the existing class, I created 2 separate classes. One with the playlist, and one for playing a single track. Please see the README or code for more information.

@sneljo1
Copy link
Contributor Author

sneljo1 commented Feb 10, 2021

@imsingh Are you able to take a look at this? Or should I make a separate package from it, as there are some API changes?

@imsingh
Copy link
Owner

imsingh commented Feb 10, 2021

Hi there. Thanks for the pull request. I'll take a look over weekend.

@imsingh
Copy link
Owner

imsingh commented Feb 15, 2021

@Superjo149 I like the idea. I'd be happy to merge this. Can you create a demo app also so that I do a final check?
I'm also okay if creating a new package helps you with your project.

@sneljo1
Copy link
Contributor Author

sneljo1 commented Feb 18, 2021

@imsingh I am already using it here, https://github.com/Superjo149/auryo/blob/392e94ebebb13670ddc96ff13f90739dc71f6490/src/renderer/epics/audio.ts#L31> But the branch itself is very much a WIP, so you will not be able to easily test it. I can maybe see to create a small dummy thing for it if you want.

@sneljo1
Copy link
Contributor Author

sneljo1 commented Feb 18, 2021

I have added a super simple example to illustrate and possibly test.

@sneljo1 sneljo1 requested a review from imsingh March 4, 2021 12:20
@imsingh
Copy link
Owner

imsingh commented Mar 4, 2021

@Superjo149 Hi. I understand you want it to be merged. It's likewise. I had pretty rough days at my work. This weekend, I'll merge it, update the companion app and release a new version. Just a little longer.

Thanks for your patience and work!

@sneljo1
Copy link
Contributor Author

sneljo1 commented Mar 4, 2021

@imsingh Allright, thanks. No worries, it was just a friendly reminder 🙂

@imsingh imsingh merged commit 98ba3b8 into imsingh:master Mar 7, 2021
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.

2 participants