Attention is currently required from: Edward O'Callaghan.
1 comment:
File board_enable.c:
Patch Set #3, Line 1097: dev = pcidev_clonedev(dev);
The direction should be fairly clear, I rebased the follow on patches to give clarity. You realise every time you change your mind here I need to keep wasting time rebasing.
How you organize your patch trains is up to you. If constant rebasing is an
issue, then don't rebase constantly. Are you really making reviewers responsible
for necessary changes during review here? Can you please tell me in your
own words what is the point of reviews?
Give actionable feedback, you wanted it renamed, then you changed your mind but agreed that 1 in the function name will do it now your comment is sort of high-Z.
Sorry, your patches look like you are spending little time to write them. So
please don't expect me to take much more time to write and repeat endless
explanations. I changed my mind because my original suggestion was based on
the idea that your original naming choice (pcidev_get_dev()) made sense which
it made little. I'll try to do better in the future and not to run into that
pitfall again.
The change here is not particularly controversial, it is just isolating libpci stuff into pcidev.c which is pretty logical - look in pcidev.c for libpci stuff. So I don't know what you mean by "I have to look in another file", do you want the whole project in one file?
I fear, we are talking past each other. What I wanted to say is that before
this patch, when reading the code in `board_status.c` I could see what the
code does: it queries function 1 of the same PCI device. Now, after this
change, when reading the code in `board_status.c` I can't see that anymore
because the fact that it's about function 1 is hidden in another file. Putting
`function` in the API name could fix that. Other languages than C might not
have that problem, AFAIR you know Ada: There you could just write that parameter
name in the call e.g. `pcidev_getdev(dev, function => 1)`. But in C all we can do
is to choose good function and variable names. (Or add code comments, but I
kind of hate those (not checked by the compiler.))
To view, visit change 59276. To unsubscribe, or for help writing mail filters, visit settings.