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

[ACHIEVEMENT] ONE OF THESE THINGS JUST DOESN'T BELONG HERE #186

Open
ZimGil opened this issue Nov 10, 2018 · 1 comment
Open

[ACHIEVEMENT] ONE OF THESE THINGS JUST DOESN'T BELONG HERE #186

ZimGil opened this issue Nov 10, 2018 · 1 comment

Comments

@ZimGil
Copy link
Member

ZimGil commented Nov 10, 2018

ONE OF THESE THINGS JUST DOESN'T BELONG HERE

ACHIEVEMENT

Achievement for getting your Pull Request locked for conversations (Off-Topic / SPAM)

It's pretty straight forward, kind of an Easter egg Achievement for the odd cases of Off-Topic / SPAM PRs.

Reference

It's a Jeff Dunham (and Peanut) reference to a Sesame Street reference for Unusual or Exceptional things.

My suggestion for metadata:

name: One of these things just doesn't belong here

image: https://goo.gl/images/tqLvAR

short: Jeff is Jeff, Heff is Heff and CCHHHHHEEFFF is CCHHHHHEEFFF!

description:
Your pull request conversation was locked because it was Off-Topic or SPAM, and it really doesn't belong here

GitHub API

Here you can find GitHub's API for the Pull Request REST response,
you can see the following properties:
locked and active_lock_reason which should be added to Achieveibits pullRequest object


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@thatkookooguy
Copy link
Member

thatkookooguy commented Nov 10, 2018

Hi @ZimGil !
Thanks for the achievement idea! It sure made me laugh :-)

I love this idea but I think it's problematic. We should keep this issue open (for future reference) but I'll try to explain why it's not something for right now.

Basically, you can lock conversations, but the only data you get on the PR object is locked: true and the reason, which is one of 4 options.
Take a look at the following scenarios:

  1. A user opened a pull request, argued and cursed on it and a maintainer locked the conversation

  2. A user opened a pull request and two of the repository team members starts a long conversation on the pull request. The project owner decided to lock the conversation.

  3. A single user in a single repository locked their own conversation

  4. A user that opened a pull request was part of a conversation that was locked on their PR, but didn't do the thing that made it locked.

Those are all reasonable scenarios. But I don't think the user should get the achievement for most of them:

  1. This is the classic scenario. This is obviously correct and the PR Creator should get the achievement
  2. In this scenario, I think the PR creator had nothing to do with the conversation locking. Maybe here the two contributors should get the achievement
  3. Here it's clear that the user passed the achievement's "test", but shouldn't get an achievement.
  4. Here, the user is part of the conversation but it's not their fault necasserily.

So, what I'm trying to say is:

  • The achievement can't be related to the person who caused the PR to be locked, which sounds odd (user X got an achievement he had nothing to do with just for openenig a PR while someone who was clearly the person to get the achievement didn't get it)
  • This is a "bad" achievement. Most of our bad achievements are related to "not the best" code practices. Locking the conversation* is more of a "your not nice" achievement. I think we should avoid those kind of achievements (at least while we still have planty of room for good \ neutral achievements
  • We don't pass that data to the achievibit PR object. It should be an easy fix to add it, but since we're just about to move to the 2nd version (which will hopefully add support for GitLab and BitBucket), this achievement also sounds less common.

We might implement this later on when most of the common things will be taken, but I think we can make plenty of achievements with more common code review specific data :-)

Try and look at data that is more common in PRs and work with that for now (and also data we already collect in our object :-)).

If you need help or have any questions, don't esitate to send me a message!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants