feat(programmatic-interface): export a function to enable running an octoherd script against a repository list #220
Conversation
… a provided list of repositories for octoherd#219
| @@ -2,6 +2,7 @@ import enquirer from "enquirer"; | |||
| import chalk from "chalk"; | |||
|
|
|||
| import { resolveRepositories } from "./resolve-repositories.js"; | |||
| import { runScriptAgainstRepositories as runScriptAgainstResolvedRepositories } from "./run-script-against-resolved-repositories.js"; | |||
There was a problem hiding this comment.
i dont love this naming, but starting with something that i figured we could discuss.
the extracted bit seems more like the bit that would be appropriate to refer to as runScriptAgainstRepositories. if you agree, this would also be the name of the new public export, so consider that while deciding if this name feels right.
without further changes, that name would obviously conflict with the current name of the caller of the extraction. do you have thoughts on an alternative name for the caller based on the other responsibilities?
There was a problem hiding this comment.
I agree that runScriptAgainstRepositories is the right name for the exported method. Not sure why I used against at the time, I think it's a direct translation of how you would say it in German. If you think there is a better term in English we can use something else, now is the time.
If you like we can rename the existing runScriptAgainstRepositories() method and file name to something like loadRepositoriesAndRunScript, to express that the method loads the repositories and then passes runs the script against each repository?
There was a problem hiding this comment.
Not sure why I used
againstat the time, I think it's a direct translation of how you would say it in German. If you think there is a better term in English we can use something else, now is the time.
i think "against" is appropriate here, so no concerns with that. just trying to be clear with the separate parts as we tease things apart
If you like we can rename the existing
runScriptAgainstRepositories()method and file name to something likeloadRepositoriesAndRunScript, to express that the method loads the repositories and then passes runs the script against each repository?
i like that. that better captures the multiple steps. it looked like some of the repo resolution logic was added later, so the original name probably made sense at the time, but i think this works better for what it has grown to
| ); | ||
|
|
||
| try { | ||
| if (octokit.log.setContext) { |
There was a problem hiding this comment.
Logging for Octoherd is relevant for reports, ...
I think for the programmatic interface we might not need to take any of this into account. I think just using a standard @octokit/core constructor should work, but users of the programmatic interface can pass in their own customized instance of course, e.g. if they want throttling or retries.
i made this conditional on being present, so the reporting behavior is still possible when the octoherd instance of octokit is used. i agree that targeting use with any instance with a base of @octokit/core would be ideal for general usage. open to alternative thoughts on approach here if you have any.
|
i also realized that i targeted an |
3c7fef4 to
574989f
Compare
…octoherd script against a repository list
I'd say a |
yeah, i meant to call that out again in this thread, but forgot. thanks for confirming after we merge this, i'll open a draft PR to promote |
updated (after realizing that deleting the target branch before changing in the PR causes issues. resolved now though) |
|
🎉 This PR is included in version 5.1.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
for #219