Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Being able to attribute people on issues for doing non code contributions is amazing. It motivates people to review, manually test things, or just do some general research.
One pain point right now is that the committers of a project need to actively check people, who haven't uploaded patch files.
After some discussion with some people I propose to change this to be opt out instead of opt in. Here are some reasons:
- First time contributors are way easier motivated when they get attributed vs. when they don't
- More experienced people get annoyed when they aren't attributed.
- Noone ever got hurt by being attributed ;)
- According to https://www.drupal.org/about/values-and-principles these principles apply here:
- Everyone has something to contribute
- Be constructively honest, and relentlessly optimistic: We assume people wanted to contribute something helpful, so we should grant them
- Be sure to have fun: I guess for some people it is part of the fun to get some attributions
Comment | File | Size | Author |
---|---|---|---|
#27 | Screen Shot 2019-01-30 at 11.43.20 AM.png | 70.73 KB | drumm |
Comments
Comment #2
justafish👍👍👍
Comment #3
alexpottThis is how I try to work when I commit a patch. By assuming all comments are credit worthy first and then looking for reasons why not to give a credit.
I think that
from the values document applies here too.
Comment #4
alexpottThe committers have a template response for unhelpful review or RTBC comments that outlines some of the reasons we don't give credit.
Comment #5
alexpottOne problem this does not address is the fact that a contributor who has spent days on a patch gets exactly the same reward as a person who spends 10 minutes on a constructive review. I'm not sure how to fix that but changing the default to everyone being auto-credited might make that worse.
Comment #6
alexpottAnother thought is that if we make this change the person writing the comment ought to be able to opt-out from credit themselves.
Comment #7
alexpottTagging with the tag.
Comment #8
justafish> a contributor who has spent days on a patch gets exactly the same reward as a person who spends 10 minutes on a constructive review
Self-selected story points? 1 (default, comment that drove the issue forward) , 2 (short patch review), 3, 5, 8, 13, 20, 40, 100
Comment #9
darvanenSeems like a good idea, though I have a couple of worries:
Just my 2c.
Comment #10
jhodgdonRegarding #5, this has always been a problem with numeric tallies of patch credit. It's also true that the person who writes a one-line patch to fix a comma in documentation gets credit for 1 patch, whereas the person who spends weeks on a patch also gets credit for 1 patch.
I'd also like to point out that the checkboxes are used for 2 things:]
a) Creating the patch commit message. This is a temporary, JavaScript use of the checkboxes, and affects what goes into the Git repo, if the committer uses the auto-generated message.
b) Giving people credit for having contributed to fixing an issue, which goes on their user profile, company profile possibly, and I believe affects companies' order of being shown on various Marketplace listings on drupal.org.
So, in principle you could have different credits for (a) and (b). Nonetheless, for both uses it is probably true that everyone from the person who reports the issue through the reviewers deserves credit by default, and I agree that unchecking should be the "I have to think about it" option for maintainers.
Comment #11
Mile23I don't think there's a disparity between one line of code vs spending a week on a thing (as per #5). If you can take on the task then do it. Also, one line of code can be the result of a lot of analysis that might not have taken a week, but requires a lot of specialized knowledge.
I've gotten through a few doors by having a low drupalcores score, but I acknowledge that it's a not all that great a metric and work to find a chance to explain why once I'm through that door.
In the Examples project, credit goes to anyone who contributed in any way that ended up defining the problem or changing code. Also if you want to be a comaintainer that's not hard at all. :-)
So basically: Yes, opt out instead of opt in, because as a maintainer I'm making a better mistake if I neglect to change the checkboxes. Once the commit is pushed, it can be hard to amend the commit message.
Comment #12
alexdmccabeI think this is a great idea!
Comment #13
catchOverall it seems like the right balance to me to flip the default.
The commit message orders by number of patches/comments submitted which still gives some indication of the main contributors to an issue (as much as anything can). We could also automate tracking of things like patch vs. no patch, authorship of change records, updates to issue summary if we really wanted to. My only concern here with 'credit everyone by default' is the issues with 30-60 people on, but they are hard to do anyway regardless.
We have 2,200 Drupal 8 patches stuck at 'code needs review', so I would not want to tweak commit credits in a way that would encourage people to post more patches and do less reviews.
The fact that we assign contribution credit on d.o when an issue is closed is related to this as well - there's a lot of latent contribution credit in the issue queue which is not reflected on Drupal.org until issues are RTBCed and committed. Also duplicate issues tend not to receive credit at all unless the duplication is discovered before one of the issues is closed, and credits are manually moved over.
There are literally thousands of uncredited patches and reviews in the issue queue all the time, so the weighting of how credit is distributed for one particular issue seems very minor in the scheme of things.
For an example:
Person A works on a patch for 10 hours. No-one reviews it. They have to keep re-rolling it (not actually constructive work) and maybe maintaining the patch in composer for the site they're working on (also not constructive work). And they get 0 contribution credit for all this.
vs.
Person A works on a patch for 10 hours. Person B reviews it. Person C reviews and commits it. Person A gets 1 commit credit, so do B and C.
So commit credit is already skewed massively away from the quantity of work performed but is tried to whether that work is realised which is not necessarily an indication of the intrinsic value of the work at all.
There's not a finite amount of contribution credits to distribute that people need to compete for, there is an infinite amount of contribution credits, and our bottleneck is in distributing enough of them (i.e. closing issues).
Comment #14
wturrell CreditAttribution: wturrell commentedI'm a bit sceptical about this (on core, anyway). I think the annoyance or "resentment" (for want of a better word) at not being ticked when a patch is committed will merely be replaced by similar feelings if a user is unticked instead. Also these things matter to some people way more than others: the people who feel entitled to a commit credit will probably complain either way around.
(I'd find it interesting to see a breakdown of how many reviews, patches or comments I'd done - do I think it would be a sensible use of anyone's time building that? No.)
For me, a bigger issue is "Credited on X issues fixed in the past 1 year" on user profiles. Why not at least display an all time total as well? It's inconsistent with the "Projects" heading below which lists all commits ever. Similarly the threshold for Documentation edits. (I'm guessing there are issues for both of these but I have to catch a train...)
Comment #15
hestenet@wturrell - all time totals were actually added to user profiles by @B_man in our last sprint. Just use the 'view all issue credits' link at the bottom of your credits list.
EDIT: @B_man also added that for organization profiles.
Comment #16
justafishInstead of opt-out, how about if someone uploads a .patch file then they can also add credit to any other users? e.g. Someone helps me in Slack with a patch, or made a really useful comment further up, then as the patch author/updater I can take responsibility for adding credit.
Comment #17
dwwGenerally +1.
My only concern is can we please change our default commit message template to move all the names *after* the thing that was changed?
#2323715: [policy, no patch] Determine format for commit credit for individuals/organizations/customers
#2811033: Discuss if git commit messages should be multiple lines
This change is going to make a bad problem much worse. Sticking to our legacy commit message conventions is a terrible reason not to start crediting more people. Can we please finalize a decision over there prioritize getting that out, too? :) Let's do both.
Thanks!
-Derek
Comment #18
dwwComment #19
DamienMcKennaFYI the change notice says:
The date needs to be updated :-)
Comment #20
hestenetWhoops! Thanks @DamienMcKenna - fixing.
Comment #23
drummThis has been deployed.
Note that credit is saved whenever a maintainer comments. For example, hestenet’s last comment saved the previous set of people getting credit with the old opt-in strategy. As I make this comment, I’m going ahead and crediting everyone myself, in the spirit of this issue.
If someone were to comment after this deployment, before I loaded the page, they would get their checkbox checked for the maintainer to save on the next comment. This matches the previous behavior of commenting with a file upload.
Comment #24
alexpottYay that this landed but there's a slight tweak that we need. Once an issue is fixed we shouldn't auto-credit people. Other comments like https://www.drupal.org/project/drupal/issues/3028319#comment-12948315 get credit which is not the intention.
Comment #25
hchonovand the first bug regarding this was found - #3029478: People appear to get credit for commenting on fixed issues.
P.S. I've just demonstrated it here as well :).
Comment #26
catchRe-opening.
Comment #27
drummCredit is not assigned until a maintainer comments. I posted a few more details in the followup issue, #3029478: People appear to get credit for commenting on fixed issues.
For example, here is a screenshot of https://www.drupal.org/u/hchonov:
There is not credit assigned for this Drupal.org customizations project issue.
Since we have a good followup issue, setting this back to Fixed.
Comment #28
jonathanshawThere is an elephant in the room that no one seems to have discussed: creditjacking.
I tried to find a Drupal agency for a small project and looked at Drupal.org's rankings of agency contribution. Once you skip past the really big agencies with large staffs at the top of the rankings, which specialise in large projects, you start finding some agencies that do handle smaller projects (typically Indian agencies). If you look at what the contributions are that have caused them to rank so highly in these Drupal.org rankings, you quickly discover that they are all engaged in gaming the system, what we could call 'creditjacking'.
The most common strategy is coding standards: detect coding standards violations in modules and then post an issue with a simple patch for a particular CS issue. You can train an intern to bang out many of these a day. It's not much help to our community, but it's also not too toxic a behavior.
Another one is screen shots. These trigger automatic issue credit, so I've seen an agency that specialised in going round issues taking screenshots and uploading them. Rarely were the screenshots much use, but they were not too toxic either.
All this changes with this patch. You're going to seen an explosion of trivial comments on issues that are not very helpful but on-topic enough that it's hard to explicitly sanction them. Comments that might me welcome from a newbie become toxic for the community at large when done industrially by those trying to game the system.
The result of this trivia-wave is that notifications for issue activity (e.g. drupal.org/dashboard) will become even less useful. The only way of getting a reviewers attention is going to be slack or IRC.
Comment #29
hchonov@jonathanshaw, yep you are right. While I don't really want to take a side on this, I've seen already people having only one single comment on issues like :
which are not really helpful comment and it would not be fair for them to get credits.
I think that there is something that we might do about this and it is:
2. is doable, because we already have the labeled data - all the fixed issues and the credits given. Sure there are mistakes in the data where a credit was not given, but this is not bad as one might think as it prevents overfitting the model. The initial model might not be perfect, but it will for sure be something in the middle between not giving automatically credits and giving too much credits. The model will improve as time goes, because we might force it to rebuild itself every time a committer reverts a decision taken by the classifier. Such a model would also make it possible to classify the contribution and e.g. if somebody had posted 50 patches on the issue would receive a higher credit points than someone who has posted only 1 patch.
P.S. I am familiar and have some experience, yet not an expert, with NLP and training ML classifiers for NLP, so if this sounds like a good idea, then I would be happy to participate in such an initiative.
Comment #30
alexpottThis issue doesn't change the fact that the committer has to decide who gets credit of not. But for example, on https://www.drupal.org/node/3029500 Berdir now got a credit for the rtbc. Bear in mind that the system in a previous change automatically adds credit to the committer - I think it is fair to credit Berdir here. Before a single quick comment like that would be hard to credit because you have to justify based on the content of the comment BUT the change now is that you have to justify removing credit - which for me is a good thing because then the onus is to explain why not so people get a chance to learn.
Also I think it better to be incorrect in the positive way rather than the negative way. The community has proved itself very able to identify abuses of the credit system in the past and I'm sure it will in the future. Yes this might mean that the credit will stay in the commit log but we can always remove an incorrect credit.
Comment #31
DamienMcKennaThe marketplace ranking algorithm weighs commits to core and top modules higher than those of less used modules. It is still completely within the maintainer's power to control who gets credit on an issue, so the maintainer can go through and uncheck folks who didn't contribute to the issue in a positive way. It's also worth noting that people who learn to commit minor things like coding standards and typoos can move on to fixing other, less mundane issues. Lastly, if maintainers don't want to have people needling them for minor coding standards fixes they're within their power to fix it before someone else does, and ensure that all patches keep up the code quality.
So let's not gate keep too hard here, we want to get more people into the community rather than fewer.
Comment #32
jonathanshawYour points are true @alexpott and @damienmckenna, but miss the central thrust of my concern.
The fact that it's theoretically possible for committers to not give credit for trivial comments is less impactful than the facts that
1) committers are only human, and defaults make a big difference to ease
2) this change affects the unwritten assumptions of the issue queues; the committer now needs a reason to exclude a comment
These two things taken together make it much more likely that trivial comments often in practice get credit
It's very cheap and easy to comment
Therefore credit abusers will pursue commenting aggressively
This could increase issue noise a lot
Increased issue noise is potentially very serious: it's already very hard for patches and meaningful comments to attract attention of the small group of expert reviewers critical to the project
I'm not sure this is the right place to discuss all this, but I suspect it's going to need a discussion.
Comment #33
larowlanFrom the comments here, its not clear if we're changing what we give credit for or if we're simply changing the tooling.
If we're proposing changing what we give credit for then I think there should be a broader policy discussion.
If its just about tooling, then I guess its much of a much-ness in terms of effort required to attribute credit correctly, we're just changing the defaults.
Comment #34
hestenetTo be clear, at least from the DA point of view, this was just a tooling change. We flipped the default from maintainer must opt-in to maintainer must opt-out. But in terms of the actual policies of who gets credited, we've made no changes. (That is up to the maintainers themselves)
Comment #35
alexpottI agree with the idea behind this issue and am in favour of changing to a model of assuming valid contribution. However, there are some UX concerns that need addressing. The big problem is that people use the checkbox list to determine who is credited - not just who will be. This change makes the situation worse because more people "will be" so the dual purpose is brought more to the fore. In order to make this change we need to more clearly indicate who is credited from who potentially should be.
That reason on its own might be a reason to keep this change and move forward with fixing that. However I think that the following issue necessitates a revert of this behaviour. People can re-open fixed issues and comment on fixed issues and then if a committer comments on said issue that have to sorted out the potential credit from the real credit again and this can be very tricky. For example I had to be very careful commenting on https://www.drupal.org/project/drupal/issues/3031128 which was re-opened after fixing.
I myself am not going to re-open this issue because I think it's best to run this past @drumm / @hestenet first.
Comment #36
catchYes issues like re-adding contribution credits once it's already been removed, and credit added to fixed issues makes me think this should be reverted for now too - it looks a bit more complicated than just reverting the default (which I also think is fine in principle).
Comment #37
drummNeither of these should be happening now. When a maintainer comments, that’s when credit is saved. The canonical record of whether credit is given or not is a user’s profile.
As a maintainer, if you uncheck credit for someone, that should be sticking when you save the issue. Next time you open the issue, they will remain unchecked. Unless they have commented again, which resets the default to be credited.
With #3029478: People appear to get credit for commenting on fixed issues, these defaults no longer get set for new comments on closed issues. If the issue is re-opened, that resets back to the same algorithm for defaults.
If any of that doesn’t seem to be the case, we might have a bug.
Comment #38
tim.plunkettLet's say you have a long running thread with 20+ people having commented. Currently, half of them have credit.
Someone who has commented before comes by the issue and leaves a meaningless comment.
With the current UI, there is no way to know if they already had credit before that comment and should keep credit, or if they are only just now getting credit.
The only way I have found to figure this out is to unpublish their last comment and examine the credit field to see what the new value is, before republishing their comment and leaving your own comment. But that is only an option for me because I have elevated d.o permissions, not due to maintainer permissions.
It needs to have some indicator that "this person is about to get credit when you save but didn't have credit before".
Comment #39
drummI deployed a change for #2978223: Issue credit form looks to non-maintainers like they could do something with it but they in fact can't to help make the crediting status clear for non-maintainers. See screenshots in that issue, or visit an issues in a project you do not maintain.
Comment #40
drummI added #3032181: Add “credit given by maintainer” indication to Credit & committing fieldset for #38, I think it stands well as its own issue.
Comment #41
xjmI have concerns with this change because:
Comment #42
hchonovWhat if we revert the change, but provide a way for contributors to seamlessly inform the committer in case they didn't receive a "deserved" credit?
It happens that people work on issues, but don't receive credit for their contributions sometimes. Committers have an enormous workload, so it is understandable and I think perfectly fine if sometimes there are mistakes. Especially if the issue has 200+ comments.
However a problem arises, that a contributor without a credit might feel awkward about posting on the issue and asking about the missing credit or even searching any other form of contact with the committer.
I think in this case it would be nice if there is a button on each fixed issue, which is shown only to users, who have commented on the issue, but didn't receive any credit. The button might be called -
"I feel like I should've received a credit, but I didn't."
.When a user pushes this button, then the committer will be informed.
I personally would consider this a good trade-off and as a contributor not ashamed of pushing the button :).
P.S. If a certain user misuses the button and pushes it for no reason, then the committer should be able to ban this certain user from having access to that button for a certain period of time.
Comment #43
dwwYeah, FWIW (not much, I know), I retract my +1 to this, based on watching issue queues over the last few weeks. @jonathanshaw in #28 is absolutely right. @xjm is right in #41. This change is incentivizing noise and garbage in the queues (which we already have enough of, already). Please revert. Let's please find other ways to credit and acknowledge people.
Frankly, the whole "credit" system incentivizes weirdness at some level. People do things for credit, not necessarily b/c they should be done.
Finally, speaking personally, the whole system doesn't seem to work at all. I happened to click on https://www.drupal.org/u/dww/issue-credits -- that's a joke. 2 pages? It should be more like 50. ;) I don't really care, since I don't think anyone is choosing to hire me or not based on this (completely bogus) count of my actual contributions to Drupal, but if I actually was worried or hungry for acknowledgement and having my "organization" featured prominently in some list, I'd (rightfully) be pissed.
And yeah, please let's focus on #2323715: [policy, no patch] Determine format for commit credit for individuals/organizations/customers, too. Our commit messages already suck. This makes it much worse.
Thanks,
-Derek
Comment #44
dwwAnother reason to revert this: every time @xjm and I reply at #2946294: Fix race conditions in OffCanvasTestBase I'm getting this:
"The issue_credit field was changed by another user. Please verify the updated value."
Ugh. No one is actually trying to assign or remove credit (that I know of). *sigh*
Comment #46
drummSounds like there is some consensus around reverting this, which I’m deploying now.
I’m keeping a couple followups that are good regardless. #3029478: People appear to get credit for commenting on fixed issues is relevant if people upload files to fixed issues, since we’re going back to the previous behavior of suggesting credit for file uploads. And #2978223: Issue credit form looks to non-maintainers like they could do something with it but they in fact can't helps summarize the credit status for non-maintainers viewing an issue.
Comment #47
jonathanshawIf we think that it's important to make it easier for maintainers to credit people for non-code contributions, then maybe a toggleable "Credited" flag next displayed to each comment (to maintainers) would make this easier.
It would have the effect of showing who is / is not being credited right in the context where they earn the credit, rather than the maintainer having to juggle 2 mental lists, of who's being credited and who deserves it.
I fear though that the implementation is too difficult for this to be realistic.
Comment #48
drummThat reminds me of #2925569: Make the crediting widget display a modal showing the person's comments
Comment #49
hchonovAnd what do you think about something like a "Complain button", which I've proposed in #42?
If we don't have something easy like this, then people will not complain as it is too complicated. I personally don't know if people tend to ask about the reasons they haven't been credited? We have couple of core committers on the issue, so probably they could say.
I've done this only once and still feel somehow strange about this. And I don't think that shy people would even think of doing that, but a button - this is easy for anyone.
Comment #50
drummBefore this was originally deployed, I can’t remember much complaining. I think there was some following up on the issue, especially to educate maintainers who might not be familiar with our crediting system. Using the contact form is an option too.
In core’s case, I’m hesitant to add to the maintainers’ workload.
I think #2987034: Allow reacting to comments would help on a comment-by-comment basis. And uses a UI element that open source contributors are familiar with.
Comment #51
catchfwiw I've had a handful of e-mails via the contact form asking for credit, and one or two pings in slack. In a couple of cases it was an oversight by me, in others the person should not have got issue credit anyway. I haven't noticed people following up on fixed issues much if at all, not sure what the effect of a button would be.
Comment #52
tacituseu CreditAttribution: tacituseu commentedA bit of a tangential note from another POV, but it might suggest another approach (possibly already tried and my confusion stems from remnants of that time).
Sometimes there are issues in which one argues for a solution that ultimately is not taken (or argues against the one proposed/implemented), or makes comments of non-contributing but at the same time not spammy (one would hope) nature.
There's a fieldset above the comment form with 'Attribute this contribution' text and a couple of checkboxes. At first sight I was under impression that I'd have to check something there to be included in commit message/credits and keeping it not checked would assure that I wasn't (then I "Learn more'd" and later also tried un-checkboxing myself in the 'Credit & committing' section but that didn't work either).
This might not be a common enough concern, but ideally I'd like to see 'I want nothing to do with this | Do not credit' option so I wouldn't end up in a commit of something I strongly disagreed with (so a counterpoint to point 3 in IS - it has potential of affecting blood pressure ;) or when I expect none. Alternatively and hopefully relevant to the issue at hand maybe make the commenters/contributors be the ones to opt-in/signal they expect to get credit (with whatever default makes sense for most).
Comment #53
drummI have not heard of a request to decline credit before. From a maintainer perspective, credit is can be deserved for an alternate approach - it took some effort to try out, and likely influenced the other approach(es).
Comment #54
jhodgdonI had someone request that I not credit them on a User Guide issue recently because they didn't agree with the change we eventually decided to make. I honored that request and uncredited them. Then before we finally committed the patch, we came around to something they agreed with after all, and I got their permission to credit them after all.
The thing is that person didn't want to be associated with a change that they didn't agree with. And it wasn't @tacituseu, but another contributor... Anyway, it can happen.
Comment #55
tacituseu CreditAttribution: tacituseu commentedAnother related point, there are issues having roots in the initial D8 technical debt, that either because of the technical difficulty, way the wind blows, or simply being of lower priority (but still annoying if you're in 20%), are open for years. What tends to happen is people start using their patches in production and posting poorly rolled 'backport patches', sometimes trying to be helpful, sometimes to have a d.o. link they could add to their composer file, and then others complain it doesn't apply or is missing a chunk, testbot tries to run it against HEAD and fails, cheers, thanks... all fun.
Let's assume (perhaps naively) they're not doing this out of ignorance or ill will counting on getting credit, they don't even have an option to do the decent thing (other than not posting but that's human nature for you).
TLDR: Can users self-police themselves (perhaps with some kind of 'noise flag') if given a chance or is it a lost cause.
Comment #56
mmjvb CreditAttribution: mmjvb as a volunteer commentedThat was me that declined the issue credit. Disagreed with the procedure that was followed when I reported that `--stability dev` is redundant when specifying a discrete version of the package on `composer create-project drupal-composer/drupal-project:8.x-dev`. The procedure followed was to create an issue with drupal-composer/drupal-project. Only when that issue was accepted the stability option was removed. My motivation was the witch hunt by current webmasters for perceived abuse of the credit system. Have no interest to become a target of such a witch hunt. So, I requested to remove the credit when nothing was done with my contribution to the User Guide.
Was very happy to see this later. Unfortunately, there are still people that believe you can abuse this credit system. The only abuse I see is the witch hunt and assigning importance to it, which it doesn't have.
Comment #58
vuilI totally agree with very good described (clearly) in those two sentences of @mmjvb in #56: