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

Create separate config schema for ReferenceSequenceTrack #1434

Merged
merged 4 commits into from
Nov 14, 2020

Conversation

elliothershberg
Copy link
Member

I noticed that after the (awesome) changes to the display methods, there were some fields that seemed unnecessary for the ReferenceSequenceTrack config schema.

For example, this is the config editor after the changes:

image

Probably not useful graphically, or in the config in general. In pairing, @garrettjstevens and I created a smaller base config schema for the ReferenceSequenceTrack.

Now, the slots look like this:

image

Removing some of the unnecessary slots for a ReferenceSequenceTrack.

@cmdcolin
Copy link
Collaborator

I guess the idea here is that this is very similar to a baseTrackConfig but subtle differences to avoid things like assemblyNames being able to be edited.

It would be good to make clear in the source code that this is the case with a clear comment that would be hard to miss

Alternatively we could avoid code duplication here, but otherwise it just needs to be clear that it is copied from the baseTrackConfig

@rbuels rbuels marked this pull request as ready for review November 13, 2020 18:22
@rbuels
Copy link
Contributor

rbuels commented Nov 13, 2020

good to merge as soon as you can get the build to pass

@cmdcolin cmdcolin merged commit 56dbcaf into master Nov 14, 2020
@garrettjstevens garrettjstevens added the enhancement New feature or request label Nov 23, 2020
@cmdcolin cmdcolin deleted the reference_sequence_config branch March 19, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants