[GitHub] GitHub file size for a specific branch #8262
Conversation
|
| if (!branch) { | ||
| branch = 'master' | ||
| } |
There was a problem hiding this comment.
The default branch for a repo is configurable and most repos now use main. Instead of setting branch explicitly here if it is not supplied, pass a ?ref= param in our API call only if branch is supplied. If the user doesn't supply the param we can leave github to work out the default.
There was a problem hiding this comment.
Hi @chris48s,
Good point, updated, thanks.
|
Hi @chris48s, I see that the test suite failed on my feature branch. Is it something that I can fix?:
|
Seems it was specious. I reran the failed jobs and they passed |
Ah, ok, thanks :) I am currently not working on anything on this branch. If there is something that should be changed before a merge to the master branch, I am open to suggestions :) |
|
Ta - merged. Running the full test suite for github is a sufficiently large number of integration tests that sometimes we'll hit a flaky one. You can actually filter this down further if you want. For example, if you run |
| if (branch) { | ||
| return this._requestJson({ | ||
| url: `/repos/${user}/${repo}/contents/${path}?ref=${branch}`, | ||
| schema, | ||
| errorMessages: errorMessagesFor('repo, branch or file not found'), | ||
| }) | ||
| } else { | ||
| return this._requestJson({ | ||
| url: `/repos/${user}/${repo}/contents/${path}`, | ||
| schema, | ||
| errorMessages: errorMessagesFor('repo or file not found'), | ||
| }) | ||
| } |
There was a problem hiding this comment.
This is fine, but one thing to be aware of for future reference is that we typically like to use the options key on the arg which in turn allows our http client library to handle the conditionality of the query search parameters for us. I gather there's also a need to have slightly different error texts depending on the branch, though that could also be handled with a single call too, e.g.:
return this._requestJson({
url: `/repos/${user}/${repo}/contents/${path}`,
schema,
options: { searchParams: { ref: branch } },
errorMessages: errorMessagesFor(
`repo${branch ? ', branch' : ''} or file not found`
),
})There was a problem hiding this comment.
That’s neat, I will try to remember about this syntax - thanks
Cool, thanks! I was not sure if it was better to run the whole GitHub test suite or a narrower one, now I know that it would be fine also with a narrower version 😌 |
Related issue: #7876
It is my first contribution to Shields - all comments and suggestions are welcome! 😉
Tested locally, gives expected results, some screenshots for example:
commit: 345da16
"size": 3572,
https://api.github.com/repos/badges/shields/contents/services/text-formatters.js?ref=345da16
branch
integration-docs"size": 3442,
https://api.github.com/repos/badges/shields/contents/services/text-formatters.js?ref=integration-docs
tag
server-2021-10-04"size": 3250,
https://api.github.com/repos/badges/shields/contents/services/text-formatters.js?ref=server-2021-10-04
tag
2.0.0-beta1our file is not present in this tag
https://api.github.com/repos/badges/shields/contents/services/text-formatters.js?ref=2.0.0-beta1
If we don't specify the branch/commit/tag, it searches the master branch by default.
On the Shields website locally, it looks like this:
The previous endpoint for GitHub file size still works correctly.