search

davidker / unisys

Master repository for new changes to drivers/staging/unisys and drivers/visorbus
Other
2 stars 1 forks source link

Update comments to match coding standard. #13

Closed davidker closed 8 years ago

davidker commented 8 years ago

I'm not sure if the function comments that we have are up to the coding style of the kernel. Or if it matters, I know that visornic had to have specific function style comments.

AlexanderCurtin commented 8 years ago

According to the CodingStyle document, kernel-doc style comments need to be used for kernel-API functions, it doesn't really say that they need to be there for module functions. But it might not hurt for consistency's sake.

davidker commented 8 years ago

@selltc Tim, can you look over this set of patches as well?

davidker commented 8 years ago

upstream-curtinap-kernel-doc

selltc commented 8 years ago

I added my comments to commit c8d99ccbb3718ffb8d17cc141ca099b7ebb54a82 and commit c93ce4059795840565bed34e80a6f570e506b4ea. I'll create patches that accommodate these suggestions.

selltc commented 8 years ago

Tweaked patches accomodating my suggestions: commit cd605f609c6cb276ceb172a1da515e125a65457c, commit 5b2d44813bc280cdfe4bbb1ecbdcda96b08e7c4a, commit 607d696b0e8b847147ee86c9d461b104e38b99c8. (2 of these patches preserve Alex as the author.)

selltc commented 8 years ago

BTW, the patches referenced in the preceding comment are checkpatch-clean and compile-clean.

selltc commented 8 years ago

How's this for a cover letter?:

staging: unisys: add polish to structs: visor_driver and visor_device

For 'struct visor_driver' and 'struct visor_device':
* unused members have been removed
* kernel-doc comments have been added

Also, all 3 patch comments (commit cd605f6, commit 5b2d448, commit 607d696) need to begin with (several are incorrect):

staging: unisys: include:
selltc commented 8 years ago

Note that we can NOT submit this patch-set upstream until the code for issue #19 (commit cad8a4efc1512f0c8bbb88f39a02ebf3f17e6712) is committed to Greg's staging-next branch, which it is NOT yet. But Greg HAS accepted it...

davidker commented 8 years ago

Greg updated the patches last night. I had today off, I plan on submitting the patches tomorrow. I'll fix the comments (although technically they only need staging: unisys: The include is just really really nice. I'll add that to my list of accept/rejects.

selltc commented 8 years ago

I see that the upstream-next versions of this patchset are:

I was just about to re-write the patch comments for these as-per my suggestions above in issuecomment-211703410, but then thought better. Perhaps it is easier to tweak the comments at email generation time.

davidker commented 8 years ago

I’ve tweaked them in the emails currently. Do you want me to cc on my test send and you sign off on them?

I’m planning on doing the test send in the next 3 minutes.

David Kershner

From: selltc [mailto:notifications@github.com] Sent: Monday, April 18, 2016 11:11 PM To: davidker/unisys unisys@noreply.github.com Cc: Kershner, David A David.Kershner@unisys.com Subject: Re: [davidker/unisys] Update comments to match coding standard. (#13)

I see that the upstream-next versions of this patchset are:

I was just about to re-write the patch comments for these as-per my suggestions above in issuecomment-211703410, but then thought better. Perhaps it is easier to tweak the comments at email generation time.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHubhttps://github.com/davidker/unisys/issues/13#issuecomment-211707014

selltc commented 8 years ago

Sure, if you'd like a 2nd set of eyes...

selltc commented 8 years ago

Greg committed these to staging-next 4/29: