[ACHIEVEMENT] helping hand #35
Conversation
ortichon
commented
Jan 26, 2017
ortichon
commented
- added new achievement - any reviewer who committed to PR which he's reviewing will get it.
- fixed minor typos in other achievements
ortichon
added
the
achievements
|
Not sure if you're up for it, but it might be a good opportunity to make two achievements that connect to each other. We can have the reviewer getting the one of them gets an and you can write the reviewers who committed in the creator's description, and the creator in the reviewer's who committed description what do you think? ofc you can think of whatever quote or idea you want |
|
you can grant both achievements in the same file. |
|
loved the princess bride reference :-)
|
There was a problem hiding this comment.
waiting for @ortichon answer before approving. everything looks good so far! ❤️
| var committedReviewers = []; | ||
|
|
||
| _.forEach(pullRequest.commits, function(commit) { | ||
| if (_.includes(pullRequest.reviewers, commit.creator.username)) { |
There was a problem hiding this comment.
commit.creator.username should be commit.author.username
There was a problem hiding this comment.
Also, _.includes won't work here since it checks if a value is included in a collection. might work with _.includes(pullRequest.reviewers, commit.author) omitting the username. but I'm not sure you should compare the entire object.
I think _.find can be better used here like so:
if( _.find(pullRequest.reviewers, {username: commit.author.username}) ) {}
There was a problem hiding this comment.
so who is the creator? I've seen it in other achievements so I copied.
can you please add an API for the commits object as well? the PR one is great.
👍
There was a problem hiding this comment.
Sure. added to PullRequestExample.MD. also, if you test things locally, everything is written really nicely (that's where I copy things to PullReuqestExample.MD) at <local_url>/logs
The creator you see everywhere is the pullRequest.creator. for commits, there's a commit.author, which is the person who wrote the commit, and a commit.committer, which is the person who pushed the commit.
|
|
||
| _.forEach(pullRequest.commits, function(commit) { | ||
| if (_.includes(pullRequest.reviewers, commit.creator.username)) { | ||
| committedReviewers.push(commit.creator); |
thatkookooguy
changed the title
…ment-helping-hand
|
@thatkookooguy |
|
@ortichon If you want I can quickly test it for you since everything is already set up on my computer |
|
@thatkookooguy |
- don't give the creator the reviewer's achievement - remove the brackets for `isMultipleCommittedReviewers` - fix usernames in each other's achievements
|
@ortichon I made some changes (some were corrections, and others were more opinion base (like changing tell me if you don't like the naked |
|
LGTM |
thatkookooguy
moved this from In Progress
to Individual achievements
in Get ready for 1st Milestone
