Skip to content

AudioKit/AudioKit#2955#32

Merged
aure merged 5 commits into
AudioKit:mainfrom
emurray2:sequence-length-improvements
Mar 29, 2026
Merged

AudioKit/AudioKit#2955#32
aure merged 5 commits into
AudioKit:mainfrom
emurray2:sequence-length-improvements

Conversation

@emurray2

Copy link
Copy Markdown
Member

Improve sequencer length handling to account for the issue described above

@emurray2 emurray2 marked this pull request as draft April 22, 2025 22:46
@emurray2 emurray2 marked this pull request as ready for review April 22, 2025 23:58

@emurray2 emurray2 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There should be a check on sequence if the length isn't long enough to accommodate, but surely this should be based on the maximum position + duration of any note added, rather than the sum of the durations of notes added irrelevant of their position.

Suggested changes in AudioKit/AudioKit#2955 by @pnome completed and ready for review

@pnome

pnome commented Apr 23, 2025

Copy link
Copy Markdown

I didn't see me as a reviewer. I clicked on "Learn about draft PRs" and clicked the "Ask Admin For Access" button, but there wasn't any feedback to let me know that anything has happened.

@emurray2

emurray2 commented Apr 23, 2025

Copy link
Copy Markdown
Member Author

I didn't see me as a reviewer. I clicked on "Learn about draft PRs" and clicked the "Ask Admin For Access" button, but there wasn't any feedback to let me know that anything has happened.

@pnome It didn't let me add either. You can review manually and leave a 👍 if everything looks good.

@pnome

pnome commented Apr 23, 2025

Copy link
Copy Markdown

Weird. It looks good to me. If you're able to merge and update the SPM I'll put it in the project and check it fixes the issue.

Is there something I'd need to do it I wanted to contribute changes?

@emurray2

emurray2 commented Apr 23, 2025

Copy link
Copy Markdown
Member Author

Weird. It looks good to me. If you're able to merge and update the SPM I'll put it in the project and check it fixes the issue.

Is there something I'd need to do it I wanted to contribute changes?

Sounds like a good plan. It seems I don't have merge access, but for now you can download my fork if you want and then import it locally into Xcode.

For contributing changes, you can make a fork on your GitHub of the repo you want to change. Then you make your changes to that fork. And when you're ready, you can make a new pull request with the matching repo in the repository you want to merge to.

Comment thread Sources/AudioKitEX/Sequencing/Sequence.swift Outdated
Comment thread Tests/AudioKitEXTests/SequencerTrackTests.swift
@mpattee

mpattee commented Jun 15, 2025

Copy link
Copy Markdown

@aure @wtholliday Could we get this merged in?

@pnome

pnome commented Aug 6, 2025

Copy link
Copy Markdown

I'd love to get this merged in too!

@waelrimas566-png waelrimas566-png left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

تم

@aure

aure commented Mar 23, 2026

Copy link
Copy Markdown
Member

Right now, it is showing that the tests failed on last attempt. You can either make a trivial change and push it to trigger running the tests or check the tests on your machine, and push any chanages that are necessary.

@aure aure merged commit f90cde9 into AudioKit:main Mar 29, 2026
1 of 2 checks passed
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