-
Notifications
You must be signed in to change notification settings - Fork 114
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
Detect merged/closed PR #137
Comments
Would a warning suffice or should this be an error? |
I'm not sure, I was thinking that a non-zero exit code would be useful if you were scripting this, but I haven't actually scripted it so I'm not sure... I was also thinking that maybe it shouldn't generate metadata, but again, there may be use-cases where you actually want to get metadata for closed PRs, so IDK about that either. Basically IDK, that's why I opened this as an issue, to see what people think 😁 . Maybe just a big |
It's hard to tell if it's actually merged given the way we land things, but a warning about the PR being closed is definitely doable. |
...oh right, we can potentially do a |
We can at least tell if it's merged if it's a collaborator's PR (e.g. nodejs/node#17899). Alternatively maybe look for a landed in comment. |
We can check and tell if PR is closed: landed by collaborator or merged: landed by the collaborator who open a PR. I was thinking a warning will be suffice since the metadata would be generated and non-zero. |
Currently if you run get-metadata on a closed PR, you don't get an obvious indication that it's closed.
It should probably be made clear that the PR is closed.
The text was updated successfully, but these errors were encountered: