Add go generate mention to CONTRIBUTING.md

[XaurDesu]

a WIP for the improvement of CONTRIBUTING.md as detailed by #2579 , mentioning the existence of generated files and the fact that they should be added alongside the manual part of the commit. We could probably add a few more things, but those will come along if suggestions do.

[codecov]

Codecov Report

Merging #2587 (f47debd) into master (c855eb5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2587   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files         131      131           
  Lines       11442    11442           
=======================================
  Hits        11219    11219           
  Misses        152      152           
  Partials       71       71           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[gmlewis]

I'm not sure this is an improvement.
Line 58 above already mentions this step.

The -x is completely unnecessary, as this step only modifies files that git already knows about, and therefore a simple commit and push is totally sufficient, and all the modifications will be caught.

I'll leave this PR open for now (in case there are other comments), but to me, mentioning what should have done "Before pushing commits" should actually be listed BEFORE this step, which in fact it already is.

[XaurDesu]

So, maybe should we put the information on generated files after line 58? moreso explaining the function of go generate when the concept is introduced. Or what ideas do you have regarding this?

[gmlewis]

@Parker77 - do you have any ideas about how these instructions in CONTRIBUTING.md can be improved?

For context, there was some confusion in #2577 and we are trying to minimize the friction that new contributors encounter when creating PRs for this repo. Do you have any thoughts about this?

[Parker77]

I guess in step 5 (maybe right after line 60), like @XaurDesu suggested, we could give a brief explanation of what’s happening when that command is run, along with the instruction that the resulting changes should be included in a PR, which could go something like:

The go generate ./... command will update or generate certain files, and the resulting changes should be included in your pull request.

[XaurDesu]

Sorry for the absence guys, i've been a bit occupied in my personal life as of late, coming back to the topic at hand, i think that yes, i think @Parker77 's format seems like a good idea for them. I'll be adding that exact comment to README.md once i get home from work today, if you guys don't mind. Maybe i could convert this pull request into a draft until we have a specific plan for the whole thing.

[XaurDesu]

Hey guys, do you think it could be a good idea to add a few links to the official Go documentation of the commands of interest in README.md? Also, i just realized that README.md mentions golint as a command that should be ran after fmt. but According to the golint repository, golint is deprecated, and given that it reccomends running 'vet' instead of golint, and that is a command we use already, should we change that reference?

[gmlewis]

Adding links to official Go documentation in the README is fine by me.
And yes, updating the README from "golint" to "go vet" sounds like a good idea too.

[XaurDesu]

Tried putting some comments in the style of @Parker77 's idea for them (including his description of go generate, actually) please check for any styling/conceptual problems in it, please. Also updated the go lint mentions.

[Parker77]

Thanks, @XaurDesu !
I made some comments and suggestions.

[Parker77]

Could you please revert the changes to the numbering? I know it makes sense to have the correct number in the code, but with markdown it’s often just fine to designate that each of these is a list item, which can be done with 1. This article talks about it.

[Parker77]

Could you please add line breaks (lines 61, 63, 65, 67, and 77 are all sort of long; adding line breaks may mean needing to adjust the text below each line, too) to match the rest of the file?
Could you also please delete the extra blank lines in between the lines of text that are all part of this block (i.e.: lines 62, 64, and 66)?

[XaurDesu]

Just committed the changes requested.

[gmlewis]

Thank you, @XaurDesu .
Just a couple tweaks, please.

[XaurDesu]

Just commited both changes, it should be good now

[gmlewis]

Whups, I missed a broken link in the last review.
Hopefully I've found them all now.

[XaurDesu]

Just corrected the dead link, i should probably have seen it before you honestly. Sorry for that.

[gmlewis]

Thank you, @XaurDesu !
LGTM.

Awaiting LGTM+Approval from @Parker77 then will merge.

[XaurDesu]

Hmm, it's been a few days, are we sure @Parker77 is available for code review?

[Parker77]

LGTM! Thanks, @XaurDesu !

[gmlewis]

Thank you, @Parker77 !
Merging.