search

ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
17 stars 60 forks source link

Blame #778

Open Keyrxng opened 12 months ago

Keyrxng commented 12 months ago

Resolves #758

Quality Assurance:

The diff comparison works by finding the merge_commit_sha and compares it to currentCommit.sha

Looking for opinions going forward from here in regards to identifying who broke the feature of the issue.

We can go direct and say 'who changed the code from pr a to pr b' and that's who is at fault but what if the feature of the issue gets broken by a peripheral change, how do we identify that? (The more I think about it this will likely be in the AI infused version am I right?)

Or do we approach it from a 'this was the last working version' so last man on top carries the burden?

If any of the original assignee's code was changed from the time it was approved to the current production version (using diff this should be easy to see) then the blame should no longer be on the original assignee.

Your spec sounds a lot like 'pr a to pr b' for me, so my next steps (at least for the first iteration) will be simply looking for any commits that overwrite the implementation of the issue.

I'm thinking if there are multiple contributors then it'll just need to boil down to whoever changed the most code?

netlify[bot] commented 12 months ago

Deploy Preview for ubiquibot-staging ready!

Name Link
Latest commit e7d5ff188b0f0ae970c87690298d1e1d93fe694a
Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/652b4cf2eb6fab0008b7eb38
Deploy Preview https://deploy-preview-778--ubiquibot-staging.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Keyrxng commented 12 months ago

@pavlovcik Bit busy the past few days but finally got around to it, let me know what your thoughts are when you can

0x4007 commented 12 months ago

Let's do two dot compare instead of three dot. I think it shows every file change. To be honest I'm not 100% clear on the difference but one seems to be more granular.

I think for peripheral change we can figure an improvement to the feature with a new bounty.

Not all lines of code changed are equal so that's why I don't want to "blame more" who changed the most lines of code. However in the blame list we could sort by most lines of code changed to least but I don't think we need to emphasize this. As long as they changed any lines from the merged pull request, they are a suspect.

This is cool to see currently.

Keyrxng commented 12 months ago

Let's do two dot compare instead of three dot. I think it shows every file change. To be honest I'm not 100% clear on the difference but one seems to be more granular.

The three-dot comparison shows the difference between the latest common commit of both branches (merge base) and the most recent version of the topic branch.

A two-dot diff compares two Git committish references, such as SHAs or OIDs (Object IDs), directly with each other. On GitHub, the Git committish references in a two-dot diff comparison must be pushed to the same repository or its forks.

Three dot gets the benefit of the GitHub UI prettifying it for us. In order to get a two dot comparison we'll need to handle formatting the url ourselves.

Currently: ref...ref and it works

Incoming: Use the compareChanges url with dual dots and insert the refs ourselves, but how should it be formatted?

https://github.com/Keyrxng/UbiquityBotTesting/issues/47#issuecomment-1722344751

Keyrxng commented 11 months ago

Now showing the reviewers, which PRs they reviewed as well the count being removed from hunters. Included a comparison for all merge commits and left in the example 2 and 3 dot comparisons lmk which of the two is the preference.

Where do I take it from here?

0x4007 commented 11 months ago

I fear that merged pulls and commits lists could get massive but I can't think of a better solution now.

Seems pretty solid so far.

As a heads up we are temporarily pausing merged into development until I can get my massive refactor in (I have a draft PR now)

Furthermore we plan to only accept with unit tests so if you want to try and get a head start feel free or you can standby for another week to add when jest is merged in the project