https://nf-co.re logo
Join Slack
Powered by
Hello <!channel> a question from the docs team (we...
# team-maintainers
j
Hello <!channel> a question from the docs team (well mostly myself, but it's for the team-docs stuff 😉 ) : how do you go about reviewing modules? It would be helpful to know what your procedure is for this. I'll put The outcome of this will result in a small poll, so please do say your personal preference/experience
For myself: when I review a module I first review against the specifications, by opening the specifications page (https://nf-co.re/docs/guidelines/components/modules) on one window, and on the other window the module. I then scroll through each point of the specs, and look at the module itself to make sure the module is compliant on that point. Once it's done I do a more general/scientific review, where I chaeck the tools 
--help
to ensure all the inputs/outputs are covered. Finally I do a general 'vibe' review to make sure it feels like it's it can be used in a typical 'nf-core' way
m
I would say poorly. I usually simply read over the module and forget to check the guidelines ( I go from memory ). I usually check for coding behaviour, and will sometimes run the tool itself in a container to make sure nothing major is off.
➕ 1
m
General vibe
and checking if tests are working
and that syntax looks good
this 1
e
If what you said is 1-2-3, I do 3-2
j
With AI (he said, with a due sense of trepidation), pointing it at the guidelines and any existing modules it reminds me of
😂 2
m
We probably should have a github actions to do a code review against the nf-core guidelines page with some AI. That would help a lot
👍 2
And if the docs link is valid, follow that and check other stuff
e
A necessary step that's missing, should be: review your code yourself first, after opening a PR 😛
party this 1
j
It would allow people get mostly there without too much human help. But we don't want the AI firing with every commit or the polar bears will cry
p
I have a 
/review
command that I use on MultiQC which is working super well
❤️ 1
j
A necessary step that's missing, should be: review your code yourself first, after opening a PR 😛
Are people not doing that? Silly people.
party this 2
p
But we need Claude credits to make this happen
I wrote a grant application to Anthropic this week
j
OK keep on topic people !
😁 2
'I use AI' is fine, but don't need details 😉
sorry 1
m
pigeon
😂 1
pigeon 2
n
I usually have a look at the file changes and check if I see anything that does not look right. I also heavily rely on linting and tests to check if the module is correctly written
this 2
m
I check who did the PR too
party this 1
if it's Jon, I trust more than other people
s
I look at the files changed, don't even remotely look at the guidelines 😉
But I have done many of them 😉
So I'm probably not a good example of best practice
party this 1
j
@Nicolas Vannieuwkerke
I see anything that does not look right
What do you define 'what is right' here?
n
Generally from experience and my memory of the guidelines, it's not 100% foolproof of course but I haven't noticed many issues with my approach
s
I don't look at the tool's help/git repo unless there is a specific question (like whether it needs some weird hack that is currently present, or if it can emit files directly into the working directory) If people haven't incorporated all input/outputs, then someone can add them in later if they want them
party this 2
j
So the conclusion is... the specifications/guidelines are not useful then 😆
(at least for maintainers, so far)
Unless you feel you've absorbed them all?
p
They are for reference and for AI 😆
🙃 1
But few people seem to use them as a checklist like you
m
we are the specifications/guidelines
shared a GIF
s
I will sometimes point at the specifications, for instance I've been doing that recently for combining mutually exclusive outputs or topics
this 6
j
But few people seem to use them as a checklist like you
So if I used them the most I can decide how they are structured, I hear you saying 😉
n
Yeah same, if I'm not sure or if I want to proof that a guideline exists, I will check and link to that guideline
👍 1
m
The guidelines are extensive. I will refer to them when pointing out how to handle errors for example. But reading over them each time before I do a review is not going to happen ( and the reason I suggest an AI - Perhaps an internal monthly code review by AI which makes issues would be more feasible).
👍 1
a
Basically, being able to points to guidelines helps supporting review comments and show they are objective, especially when it comes to style. Without this is would be easy to just dismiss them and argue it's just a matter of taste.
💯 5
👍 2
s
Yeah, particularly if someone is being difficult having strong guidelines is helpful
Open in Slack
PreviousNext
Powered by