search

zulip / zulipbot

GitHub workflow-optimizing bot by @zulip
Other
85 stars 65 forks source link

Fixes #140. Inactivity label not marked if PR 'needs review'. #144

Closed vibhorgupta-gh closed 6 years ago

vibhorgupta-gh commented 6 years ago

This fixes #140. There is a PR addressing the same issue #142, but due to merge conflicts and a very messy git history in an attempt to resolve the conflicts, made a fresh branch and pushed the changes all at once.

vibhorgupta-gh commented 6 years ago

142 is a duplicate PR to this, with this being preferred due to no merge conflicts, hopefully.

See discussion there.

synicalsyntax commented 6 years ago

Nice job! One last thing before merging; can you edit the commit message to fit the Zulip version control guidelines? You should be able to do this with the git commit --amend command :)

zulipbot commented 6 years ago

Hello @VibhorCodecianGupta, it seems like you have referenced an issue in your pull request, but you have not referenced that issue in your commit message(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged.

Please run git commit --amend in your command line client to amend your commit message description with "Fixes #[issue number]”.

An example of a correctly-formatted commit:

commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51

Thank you for your contributions to Zulip!

vibhorgupta-gh commented 6 years ago

It was a mistake on my part, I made the changes but never pushed them. Will do so at once.

Function.apply() would pass the client into new functions, so what would the this arguement passed into Function.call() do? They seem to be performing the same task.

synicalsyntax commented 6 years ago

@VibhorCodecianGupta Since I won't have a lot of time to do thorough reviews in the upcoming days, I took your commits, squashed them, amended your commits to match the commit discipline and properly resolve the issue, and merged it as e6720a4e. You should compare the differences between e6720a4e and the changes you implemented to understand what additional work your solution would have needed to work out.

Thanks for your hard work on this, and congrats on getting your first PR merged to zulipbot! :)