Create tool to update and validate GitHub v3 API URLs #1477
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1477 +/- ##
=======================================
Coverage 68.12% 68.12%
=======================================
Files 94 94
Lines 8454 8454
=======================================
Hits 5759 5759
Misses 1829 1829
Partials 866 866
Continue to review full report at Codecov.
|
|
Thank you, @wesleimp! Since this is a non-trivial addition and affects future maintenance, I would like to also get a review from @willnorris to make sure he is on-board with this change before going ahead with it. No rush, @willnorris - nothing is waiting on this. Thank you! |
|
I haven't done a thorough review of the generation code, but it certainly looks to be pretty clever (and probably a lot of fun to write!). But it's also rather complex and hefty (and potentially fragile), for maintaining what is a very minor aspect of our godocs. In fact, I think I started include the GitHub doc URLs somewhat on a lark, as just an extra "nice to have". I think the number of changes in the doc URLs certainly shows how out of date they are, so I'm totally supportive of whatever tools make it easier to keep those up to date. I'm just worried about the ongoing maintenance of it. What do you think of pulling it out of the main github folder, similar to what we did with the scrape package? Maybe even make it its own go module, so it doesn't get downloaded as part of the main module when someone is just using go-github, but not actually contributing to it? wdyt? |
Great idea! I like it. Thanks, @willnorris ! I'll work on that and update this PR when I've got it moved. |
|
Tool moved to "update-urls" subdir with its own "go.mod" and "go.sum". PTAL (low priority). |
There was a problem hiding this comment.
Admittedly, I still haven't taken a close look at the doc generation code, but given that it's now separated out, I'm a lot less worried about it.
Ideally, I'd split this out into two commits, one that makes the breaking changes to the few methods that got updated (like in github/users_projects.go, and you mentioned elsewhere about an update to some method receivers?), and a separate commit that updates the doc URLs and adds the update-urls package. Those are really two unrelated things, and so the commits would reflect that.
Otherwise, LGTM.
|
That makes sense, Will... thank you! I'll pull those breaking changes out into a separate PR and not merge this one yet until that one is in. |
|
This PR is now on hold until #1487 has been merged, which is a prerequisite for tests to pass. |
|
This PR now contains only changes to comments and the addition of the |
Fixes #1476.
Note that this
is a breaking API change because itfound receivers that were being passed by value instead of by reference, and these have been fixed and pulled out into a separate PR #1487.