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

Add enum class for custom tags, implement conversion to Tag #54

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

jdidion
Copy link
Collaborator

@jdidion jdidion commented Sep 19, 2023

Benefits:

  • Avoid having to use a bunch of constants (although you could do tag::CHAIN_ALIGNMENT_SCORE, etc. instead)
  • When a Tag::Other const initializer is implemented, you only need to change the implementation of From - public API stays the same.

Drawbacks:

  • Deviates from how standard tags are implemented in noodles (as constants)

In any case, you'll get a minor performance bump from converting from bytes rather than strings.

@jdidion jdidion requested a review from nh13 September 19, 2023 19:28
@jdidion jdidion added the refactor Refactor existing code label Sep 19, 2023
@jdidion jdidion requested a review from nh13 September 19, 2023 19:56
@jdidion
Copy link
Collaborator Author

jdidion commented Sep 19, 2023

@nh13 am I allowed to merge this into your branch, or should I let you do that?

@nh13 nh13 merged commit cee27d4 into feat-44-sam-flags-and-tags Sep 19, 2023
4 checks passed
@nh13 nh13 deleted the tag-enum branch September 19, 2023 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants