Grant credit for all closed issues, not just fixed issues

Created on 14 March 2024, 4 months ago
Updated 8 April 2024, 3 months ago

Problem/Motivation

Drupal.org has multiple statuses for closed issues: fixed, duplicate, works as designed, etc. GitLab issues only have the boolean open/closed built in. More metadata can be added to record the reason for closing, but that does not have to be standard per-project. A maintainer could add Closed-reason:bad-idea if they like. #3254602: Using GitLab labels for issues on Drupal projects → is the issue for planning standard label use.

Currently, issue credit is only awarded for fixed issues. Credit is meant to encourage productive contributions. Fixed issues are more likely to have been helpful. Maintainers don’t have to think about giving credit for issues that are works as designed, outdated, or any other closed, not fixed status. The maintainer’s crediting checkboxes do always save who would be credited, but the credit is not awarded unless the issue gets fixed.

In addition to GitLab not having the same statuses, there are legitimate cases where an issue that doesn’t get fixed, but still needed non-trivial effort. An outdated issue might have required quite a bit of research to confirm the issue is no longer present. Right now, if the maintainer does think that effort should be credited, they would have to shoehorn it into fixed status.

Before we start migrations for rebuilding issue crediting #3322116: Contribution records revamp → , we can make the existing issue credit system act like the future system. This will get maintainers used to being able to credit anything closed; as always, they should not feel obligated to credit unproductive issues & comments. And there will be less changing at launch time.

Potential solutions

  • Stay as-is, with a group-level label for “Fixed” in GitLab. Displays of issues credited will only show issues that are both closed and have the fixed label. If maintainers use some other label, that won’t be credited. Maintainers can not remove the “Fixed” label option.
  • Credit is possible for all closed issues. We embrace GitLab’s flexibility, maintainers don’t need to use any labels, unless they want to. Recording credit for any closed issue that required non-trivial effort is encouraged.

Proposed resolution

#3254602: Using GitLab labels for issues on Drupal projects → is moving toward not requiring standardized labels. Credit will be possible for all closed issues.

Remaining tasks

If this moves forward, any credit-related place which looks for Fixed & Closed (fixed) issues would be replaced with not in project_issue_open_states() or Fixed. This includes a few places in module code and Views filters.

Since potential credit previously recorded would be awarded, likely without the maintainer knowing, all closed not-fixed issues should be bulk updated to remove that record.

Documentation should be updated, maintainers should be given explicit allowance to use their judgement and not feel obligated to record more credit for all types of closed issues. They should continue crediting productive contributions, and now have more options to do so.

User interface changes

The UI should reflect the changes.

✨ Feature request
Status

Active

Version

3.0

Component

GitLab integration

Created by

🇺🇸United States drumm NY, US

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @drumm
  • 🇪🇸Spain fjgarlin

    Since potential credit previously recorded would be awarded, likely without the maintainer knowing, all closed not-fixed issues should be bulk updated to remove that record.

    This part worries me as we'd be removing data. Many issues might have been closed and attributions might have been set, and for some of these, the attribution may be important for follow-up issues (ie: transferring credit from one issue to the other) or purely as history of the issue itself.

    --

    I think that a more simple alternative, which would not require modifying any existing data, could be to modify from now on, the list of options available to close an issue to just "Fixed", which can be renamed to "Closed".

    This way, we will still get maintainers used to having just a closed state, use labels if they feel like they're needed. We can even put a form state, that when "closed" is selected, displays a message to remind maintainers about checking attribution and adding labels if they want contextual info.

  • 🇺🇸United States drumm NY, US

    I am also not sure about removing data, but I think it may be the least bad option. We should figure out how many issues and people might be affected. We could be lucky and it won’t make too much of a difference either way.

    Since we are doing a bulk update, we could post an issue comment explaining what happened. I’m not a huge fan of that, since comments on closed issues are almost always not something I want to spend time on. But it might be the right thing to do in this case.

    I wouldn’t want to remove the closed-for-reasons statuses yet, so the migration remains relatively clean. We wouldn’t want multiple issue labels with works-as-designed, worksAsDesigned, etc. And the GitLab scoped labels will be mutually exclusive, so an issue can’t have multiple closed:* reasons, if they are migrating that way.

  • 🇺🇸United States drumm NY, US

    Looking at issues with their status last changed in the last year, “With credit” is the number of issues where the maintainer has saved credit for someone:

    Summarizing:

    • 33,046 Fixed & closed (fixed) issues have credit assigned 90% of the time
    • 15,609 Closed (not fixed) issues have credit assigned 25% of the time

    So 1/3 of closed issues are not fixed, and they have credit saved at a much lower rate. If credit for these were an awarded, 3,980 issues would newly have credit that is counted on user profiles & organization pages; that would be 13.5% more issues with credit overall.

  • 🇺🇸United States drumm NY, US

    Query used:
    SELECT fdf_is.field_issue_status_value, count(DISTINCT n.nid) issues, count(DISTINCT fdf_ic.entity_id) with_credit, count(DISTINCT fdf_ic.entity_id) / count(DISTINCT n.nid) * 100 percent FROM node n INNER JOIN field_data_field_issue_status fdf_is ON fdf_is.entity_id = n.nid INNER JOIN field_data_field_issue_last_status_change fdf_ilsc ON fdf_ilsc.entity_id = n.nid AND fdf_ilsc.field_issue_last_status_change_value > unix_timestamp(date_sub(now(), INTERVAL 1 YEAR)) LEFT JOIN field_data_field_issue_credit fdf_ic ON fdf_ic.entity_id = n.nid WHERE n.status = 1 GROUP BY fdf_is.field_issue_status_value;

  • 🇪🇸Spain fjgarlin

    The biggest problem I see with those 25% is that a lot could come from those auto-checking features we used to have (open MR, do a commit, upload a patch...).

    --

    We could leave all the statuses available and then, in the submit hook, we could change the status to "Fixed" and do the right tag via code. So, somebody could choose "Closed (work as designed)", and the code will set it to "Fixed" and add the tag "workAsDesigned".

    I also think it'll be important to notify maintainers when they select one of the "Close (...)" states, that doing this will start granting credit.

  • 🇺🇸United States cmlara

    Considering we have had issues with credit farming and the fact that anything but closed fixed was intended to be no-credit I'm -1 on using older data. There should be a changeover date in the future instead. See #3432188: [META] Employees of various companies are posting low contribution work in bulk → for relevant background.

    +1 not notify maintainers, it should be announced for weeks (not days) before any changeover is done as this system has been in place for years.

    we can make the existing issue credit system act like the future system. T

    Fundamentally for me, before #3322116: Contribution records revamp → is fully designed (we have a mock-up of the process approved) it feels off to say we are doing this to "act like the future system" if the future system may be significantly different.

  • 🇬🇧United Kingdom longwave UK

    I have no opinion on the GitLab mapping yet as I haven't really thought about it at all, but I will say that I often feel bad when closing an issue as won't fix when a bunch of work has already been done. Usually the work was perfectly valid, but something else changed to mean we didn't need that particular solution any more, and while the effort was ultimately wasted I do feel like it should still be worthy of credit.

  • 🇫🇷France nod_ Lille

    Very much in agreement with #7. +1 to granting credit on closed issue starting from a date in the future. Doing it retroactively doesn't seem fair.

    For the closed state do we all agree postponed means closed? Doesn't seem right with me.

  • 🇭🇺Hungary Gábor Hojtsy Hungary

    I would look at this a bit differently. 33,046 Fixed & closed (fixed) issues vs 15,609 Closed (not fixed) issues in the past year means that roughly 2/3rds of the closed issues got credited automatically so far and one third did not in the past year. In other words maintainers who want to keep the old behaviour would need to work 33% more to undo crediting that may now happen. Is that a fair assessment?

  • 🇬🇧United Kingdom catch

    postponed isn't closed for me, it means it's waiting on another issue.

    Core has (in general) a much higher proportion of older issues with auto-credits on them, so it would indeed be more work to have to go through and un-credit people when closing out issues. This means adding those credits in retrospectively is a definite no from me, doing it from an arbitrary point in the future is doable but would definitely mean more effort overall. There are situations where I close an issue as outdated/duplicate etc. without an obvious active issue to transfer credit to, and sometimes feels like it would be nice if those credits got counted, but it does mean more commit credit effort overall.

  • 🇺🇸United States drumm NY, US

    When/if this goes in place, I hope maintainers do not feel obligated to spend too much time crediting. I think the average closed, not fixed, issue is probably not too productive. Duplicate issues are a good example where significant triaging effort can happen, and would be good to close out the duplicate and assign credit. If the duplicate issue was a basic report with zero research, in my opinion, no need to think about crediting.

  • 🇧🇪Belgium BramDriesen Belgium 🇧🇪

    How I usually go about closing duplicates is to move and manually credit the relevant users on the parent issue it duplicated if they provided a meaningful contribution or input. But yeah, not everyone does that or is aware he can do so.

    I also agree with the fact this should not be done retroactively as I'm afraid too many people will get a credit which they don't really deserve.

  • 🇪🇸Spain fjgarlin

    The changes for counting credits would be around here https://git.drupalcode.org/project/drupalorg/-/blob/7.x-3.x/drupalorg/dr...

    We could potentially split the query in two, before a given date and after a given date. The version "before" would consider only status 2 and 7 (fixed and closed-fixed), the version "after" would consider more states.

    --

    We'll need to keep that logic when migrating to the new system, but we are not there yet.
    Not sure about the urgency of this change, but as the new system would ideally come into place this year, we can also just wait and do nothing. The new system already has the ability to grant credit on any closed issue.

  • 🇺🇸United States xjm

    #13 is definitely a part of our process. However, I do see the value in crediting WAD, or cannot reproduce issues when participants have taken the time to try to reproduce the issue or make decisions as to whether it’s by design. The bugs, smashing initiatives worked around by having triage issues.

    One thing if this would affect is that it would proportionally increase the credits for these tasks from one issue per fortnight to potentially multiple issues for fortnight. However, maybe that is appropriate.

    I think it is definitely important not to grant credit for previous issues. when we first adopted the crediting system, we did not retroactively credit all of the contributions on previous issues that were fixed. Therefore, we should also not retroactively, or accidentally credit issues that were unintentionally credited in the past.

    I do not think we should remove the various differentiated close statuses. They provide valuable metadata for our process and metrics, as well as communicating important information to any user who finds them instantaneously.

  • 🇬🇧United Kingdom catch

    One other issue here is that by convention only core committers move core issues to fixed (or at least in 99.9% of cases), which means credit happens there too. But with other statuses, more people use those, and not everyone has access to change credit - tbh I'm not sure the exact list of people who do...

  • 🇺🇸United States drumm NY, US

    Maintainers that can award issue credit are people with “write to vcs” or “maintain issues” maintainer access to the project.

    We could potentially split the query in two, before a given date and after a given date. The version "before" would consider only status 2 and 7 (fixed and closed-fixed), the version "after" would consider more states.

    We also have various Views and ad-hoc reports we’d need to update. Likelihood of getting a query wrong somewhere is relatively high. I think the data loss of removing filled out credit for closed-not-fixed issues would be better, at least technically. Removing the filled-out credit also makes a more-accurate blank slate for any of the old closed issues that might be updated.

    We'll need to keep that logic when migrating to the new system, but we are not there yet.
    Not sure about the urgency of this change, but as the new system would ideally come into place this year, we can also just wait and do nothing. The new system already has the ability to grant credit on any closed issue.

    We have pretty much the same choices when rebuilding & migrating issue credit. Waiting until migration means we don’t migrate any credit for closed-not-fixed issues, so we aren't suddenly awarding credit for old issues. We effectively do the same data removal, by not migrating part of it, to get the same result.

    I’d like to get this done ahead of the migration, since it doesn’t need to be coupled to the migration. This helps the migration be as straightforward as it can be, not bundled with changes to what is credited at the same time as the UI changing.

  • 🇬🇧United Kingdom catch

    I think the data loss of removing filled out credit for closed-not-fixed issues would be better

    I think this would be fine too - it's not maintained now at least in core so doesn't feel like data we need to keep. The comments and patches are still there.

  • 🇳🇿New Zealand quietone New Zealand

    So, what happens when a non-maintainer, someone who can not assign credit, closes an issue? If I am following this correctly it means that the credit assigned on that issue will take affect. However, that credit may be incorrect and is likely to be incorrect if assigned during the intense gaming period when it was done automatically.

    We embrace GitLab’s flexibility, maintainers don’t need to use any labels,

    I think knowing the reason an issue was closed is useful information and should be retained, as explained in #15. Also, it is vital to my work managing the change records. Anything that make that task harder I can not support.

    I've yet to find anything that explains or shows how maintainers will adjust credit in GitLab. Is there?

  • 🇪🇸Spain fjgarlin

    There is internal documentation, but as the new system is not yet live, it is in a Google doc. All new systems will have public d.o pages (created from those internal documents) once a system goes live. But for now, it is a changing system (for example, the outcome of this issue will mean that we need to do changes to the new system) so discussing bits and pieces in the different d.o issues is the best way.

    The goal is to get the new system to do everything that is possible in the current system, and once live, iterate from there.

  • 🇬🇧United Kingdom catch

    @quietone I think the 'don't need to use any labels' is on a per-project basis, so Drupal core can have a fixed list of labels, but they're defined by core committers + subsystem maintainers, not by the DA, and then contrib modules can have their own sets (possibly with some defaults?).

Production build 0.69.0 2024