Skip to content

Replaced the Seqhash sequencetype with Struct type argument instead of a plain string.. - #264

Merged
TimothyStiles merged 7 commits into
bebop:mainfrom
rkrishnasanka:refactor/seqhash-updates
Sep 11, 2022
Merged

Replaced the Seqhash sequencetype with Struct type argument instead of a plain string..#264
TimothyStiles merged 7 commits into
bebop:mainfrom
rkrishnasanka:refactor/seqhash-updates

Conversation

@rkrishnasanka

@rkrishnasanka rkrishnasanka commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

Replaced the sequenceType argument with an Enum.

Currently using the direct const type enum.

Addressing - #264

@rkrishnasanka

Copy link
Copy Markdown
Contributor Author

@TimothyStiles I have a question. Is the vanilla enum implementation sufficient or do I need to convert this into a struct type enum ?

@rkrishnasanka
rkrishnasanka marked this pull request as ready for review August 17, 2022 12:46
@rkrishnasanka

Copy link
Copy Markdown
Contributor Author

@TimothyStiles can we close this?

@Koeng101

Koeng101 commented Sep 6, 2022

Copy link
Copy Markdown
Contributor

@TimothyStiles fix the discord hooks!

@Koeng101

Koeng101 commented Sep 6, 2022

Copy link
Copy Markdown
Contributor

Other than that, LGTM. Lots of little linter changes that I'm cool with

@TimothyStiles

Copy link
Copy Markdown
Collaborator

@TimothyStiles fix the discord hooks!

on it now.

@TimothyStiles

Copy link
Copy Markdown
Collaborator

@rkrishnasanka can you merge my PR into your local fork? Needs to be done for CI/CD to work.

rkrishnasanka#1

@rkrishnasanka

Copy link
Copy Markdown
Contributor Author

@TimothyStiles I'm not sure why this is still failing. I merged your PR 😕

@TimothyStiles TimothyStiles left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@rkrishnasanka this is an excellent PR. Thanks so much for migrating Poly away from ioutil which is being sunset by the official Go team. I'm going to re-add the go linter and merge!

@rkrishnasanka

Copy link
Copy Markdown
Contributor Author

No issues ! Let's keep this rolling, let me know if there are other issues you need help on.

@TimothyStiles
TimothyStiles merged commit eba6b72 into bebop:main Sep 11, 2022
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.

3 participants