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

Update getSequences to also return ranges with sequence behavior #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MImranAsghar
Copy link

No description provided.

@stephenwf
Copy link
Member

Change looks good. This will be a breaking change for the UV as it sometimes casts the output of getSequences() like:

const sequences: Array<Sequence> = manifest.getSequences();

I know #79 brings the interfaces closer, but do you think we should align the interfaces so we have something like:

interface SequenceInterface {}

class Sequence implements SequenceInterface {}

class Range implements SequenceInterface /* other interfaces? [...] */  {}

class Manifest {
  // ...
  getSequences(): SequenceInterface[] { /* ... */ }
}

cc @edsilv @demiankatz

@MImranAsghar
Copy link
Author

MImranAsghar commented Jan 22, 2021

Hi @stephenwf , just to make sure i understand correctly, in this case sequence interface would list out the definitions for all functions and types needed for sequence and be implemented by the Sequence and Range classes, and the implementation of those functions like getCanvases would be within each range.ts (like #79) and sequence.ts. So in that case we can use the sequence interface as a type instead of Sequence | Range for getSequences()?

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