lib/test: add initial prototype for wiby --test #3
Conversation
andrewhughes101
force-pushed
the
prototype
branch
from
a80429a
to
0db2338
Compare
| if (process.argv[2] === 'test') { | ||
| test() | ||
| const args = require('yargs') | ||
| .option('test', { |
There was a problem hiding this comment.
Should we consider test / result to be commands instead? e.g.
wiby test --dependent=[git url or github repo ref or package name from npm] --wait-for-results
wiby result --dependent=[git url or github repo ref or package name from npm]
This does have the downside that you can't --test --result like you do now, but maybe --wait-for-result or smth, which keeps polling for results, is good enough.
The upside is that this allows structuring the project with Yargs.commandDir(). I find that commands are also a nice way to expose and mirror the behavior between a package being a lib and a cli, i.e. require('wiby').test({ dependent: '@pkgjs/nv' }) can be equivalent to npx wiby test --dependent=@pkgjs/nv.
There was a problem hiding this comment.
Covered by #7
| const blobSha = await github.createBlob(owner, repo, encodedFile) | ||
| const newTreeSha = await github.createTree(owner, repo, treeSha, blobSha) | ||
| const commitSha = await github.createCommit(owner, repo, message, newTreeSha, headSha) | ||
| await github.createBranch(owner, repo, commitSha, branch) |
There was a problem hiding this comment.
I haven't looked in detail, but I think it might be possible to do all of the above in a single API request: https://octokit.github.io/rest.js/v18#repos-create-or-update-file-contents.
There was a problem hiding this comment.
Covered by #9
| const file = JSON.stringify(packageJSON, null, 2) + '\n' // assumes package.json is using two spaces | ||
| const encodedFile = Buffer.from(file).toString('base64') | ||
| const message = `wiby: update ${dep}` | ||
| const branch = `wiby-${dep}` |
There was a problem hiding this comment.
Does dep need sanitization here? Is @ an allowed character in a git branch (in case dep is a scoped package).
Should the branch and message also include some ref back to the hash / branch / PR of the dep being tested?
|
|
||
| const fetchRegistryInfo = module.exports.fetchRegistryInfo = | ||
| async function fetchRegistryInfo (dep) { | ||
| const resp = await fetch(`https://registry.npmjs.org/${dep}`) |
There was a problem hiding this comment.
We should use pacote for this, so that it can support custom registries. Happy to address this as part of #6.
| const urlRegex = /github.com\/([^/])+\/[^/]+/g | ||
| const resp = await fetchRegistryInfo(dep) | ||
| let org, repo | ||
| if (resp.repository && resp.repository.url) { |
There was a problem hiding this comment.
resp.repository can also be a string (the URL). And in this case git-url-parse is definitely handy.
There was a problem hiding this comment.
Covered by #10
|
|
||
| const getOrgRepo = module.exports.getOrgRepo = | ||
| function getOrgRepo (url) { | ||
| const repoOrgArr = (url.split('github.com'))[1].split('/') |
There was a problem hiding this comment.
I find https://www.npmjs.com/package/git-url-parse handy for things like this, esp. if we're going to support Github Enterprise.
There was a problem hiding this comment.
Covered by #10
There was a problem hiding this comment.
I agree with @dominykas, let's land this and then iterate.
I also like the quite like the idea of the entry point being wiby test rather than wiby --test - I'll open an issue to follow up on that.
|
Thanks for the reviews @dominykas @BethGriggs, Im going to land this, any outstanding items in the reviews I'll turn into new issues so anyone can work in parallel on them. |
|
🎉 This PR is included in version 0.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Intial protoype of tool, as demoed in the package maintenance meeting yesterday.
This PR contains the functionality to run
wiby --test dependent_urlwhich will take the HEAD commit from the repository the tool is being run from, and then update the package json of the dependent module provided to the tool as dependent_url.