-
Notifications
You must be signed in to change notification settings - Fork 0
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
KANBAN-504 seq retrieval #2
Conversation
This enables relevant IDE tooltip hints in VS Code (and presumably other IDEs) and thus more efficient coding.
@mluypaert I don't know much about this project and I haven't even tried to understand the details of each function, but from a very high-level perspective it looks good to me, considering how readable and organized the code is and how you structured config files and documentation. |
Just checked out the repo and it looks clean and well commented. The seq retrieval code also looks good! Thanks @mluypaert |
@mluypaert This looks really good -- no problems at all with the structure and your comments are great. I think we'd have no problem passing this around between developers if necessary. |
In the checks before merging is allowed it would be nice to have flake8 or something of that type that has to pass too. |
@ianlongden Good point, I was planning to potentially add that at some later point, but having had a closer look at flake8 it was easy enough to just enable it straight away, so I've added that as well now. Thanks! |
Hey @christabone, would you mind reviewing this PR?
Not as much whether the code would be functional (I assured that through testing) but more if the code itself is readable to you, whether the repository folder structure is sensible to you, whether the development, typing and documentation guidelines I've written in the README make sense etc.
The goal is to make this repository accessible to other (AGR) developers should they wish to contribute or need to take over future development. So your review should essentially boil down to the question "If I were to hand over this repository to you to continue development, would you be comfortable to do so with the current state of files, guidelines, rules and checks? And if not, what's missing?". I still need to add unit testing (and add that to the PR validation) so I'm aware of that, but otherwise I think everything in this PR should be ready to be worked on by anyone with the relevant subject knowledge.