search

bebop / poly

A Go package for engineering organisms.
https://pkg.go.dev/github.com/bebop/poly
MIT License
663 stars 70 forks source link

`Feature.GetSequence()` always returns a nil error value #352

Open carreter opened 11 months ago

carreter commented 11 months ago

https://github.com/cachemoi/poly/blob/10b8f398c4272700dd27a177af244d6b531d2582/io/genbank/genbank.go#L129-L157

Should either not have an error in its signature or actually return an error.

paleale commented 10 months ago

Hi! You don't mind if I'll take that? What if I have minor corrections to 'CONTRIBUTING.md' about URLs actualization, could I bring them with?

carreter commented 10 months ago

If you could the other changes in a separate pr, that'd be great.

This is currently blocked by @Koeng101 's PR! Try and merge it into his ioToBio PR instead.

paleale commented 10 months ago

Other proposed changes are just 2 lines, sure need a separate pr?, like:

-You can learn how from this *free* series, [How to Contribute to an Open Source Project on GitHub](https://egghead.io/series/how-to-contribute-to-an-open-source-project-on-github).
+You can learn how from this *free* series, [How to Contribute to an Open Source Project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github).

-You can also check out [these](http://makeapullrequest.com/) [tutorials](http://www.firsttimersonly.com/).
+You can also check out this [tutorial](http://www.firsttimersonly.com/).

First link was working but with 2 redirections, latest is the final destination at the moment. Second one (makeapullrequest.com) just invites to go via the same egghead.io/courses link, maybe could've been removed as a duplicate then?

Try and merge it into his ioToBio PR instead

Sure, will try.

carreter commented 10 months ago

Ah gotcha. I'm generally opposed to drive-by changes, but this seems fine since it's really small and in documentation files only.

Thank you so much for taking a crack at this issue by the way!

paleale commented 9 months ago

Hi again, sorry for being lost for some time. Does the same function in io/polyjson have to be ~un-errored:) too? I see that it's basically doing the same, but has some unclear todo action.

Koeng101 commented 9 months ago

~un-errored:) too? I see that it's basically doing the same, but has some unclear todo action

That's... uh unclear I think. Personally I think we should nuke polyjson - it doesn't accomplish much that Genbank does better. But basically, you are correct that it theoretically should be un-errored. I think polyjson it hasn't gotten a lot of love because nobody uses it (they just use JSON version of genbank)

github-actions[bot] commented 7 months ago

This issue has had no activity in the past 2 months. Marking as stale.