Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support metadata query by userid #397

Closed
wants to merge 1 commit into from
Closed

Conversation

@srl295
Copy link
Member

@srl295 srl295 commented Mar 21, 2020

git node metadata @srl295 will show up to 10 active PRs and give the current status.

I'm not a frequent contributor to node. But, sometimes I have a couple of different PRs in the works. This helps me find out what the current status is.

"git node metadata @srl295" will show up to 10
active PRs and give the current status.
@codecov
Copy link

@codecov codecov bot commented Mar 21, 2020

Codecov Report

Merging #397 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #397   +/-   ##
=======================================
  Coverage   76.28%   76.28%           
=======================================
  Files          21       21           
  Lines        1480     1480           
=======================================
  Hits         1129     1129           
  Misses        351      351           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32c7b48...ad4dada. Read the comment docs.

await getOneMetadata(nargv, cli, request);
cli.separator();
}
return {}; // ?

This comment has been minimized.

@srl295

srl295 Mar 21, 2020
Author Member

not sure what the return should be here

This comment has been minimized.

@joyeecheung

joyeecheung Mar 30, 2020
Member

I think a better way to handle multiple requests is to add a different method for it, which calls the old getMetadata, and then call it from the command handler, instead of overwriting the method that's used to querying single requests. That also helps you avoid the nargv.assignee tinkering.

return runPromise(getMetadata(merged, cli)
.then(({ status }) => {
if (status === false) {
throw new Error(IGNORE);
}
}));

In addition you may want to update the fs.writeFileSync call in getMetadata to fs.appendFile instead.

await getOneMetadata(nargv, cli, request);
cli.separator();
}
return {}; // ?

This comment has been minimized.

@joyeecheung

joyeecheung Mar 30, 2020
Member

I think a better way to handle multiple requests is to add a different method for it, which calls the old getMetadata, and then call it from the command handler, instead of overwriting the method that's used to querying single requests. That also helps you avoid the nargv.assignee tinkering.

return runPromise(getMetadata(merged, cli)
.then(({ status }) => {
if (status === false) {
throw new Error(IGNORE);
}
}));

In addition you may want to update the fs.writeFileSync call in getMetadata to fs.appendFile instead.

* @param {Object} request
*/
constructor(argv, cli, request) {
const { assignee, owner, repo } = argv;

This comment has been minimized.

@joyeecheung

joyeecheung Mar 30, 2020
Member

I think it's probably enough to just pass these directly here instead of through argv - this is very far away from the command handler now and argv isn't too meaningful here.

@codebytere
Copy link
Member

@codebytere codebytere commented Jul 14, 2020

@srl295 do you think this is something you still want to work on?

@srl295
Copy link
Member Author

@srl295 srl295 commented Jul 15, 2020

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Oct 13, 2020

This PR is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Oct 13, 2020
@github-actions github-actions bot closed this Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.