danger / swift

⚠️ Stop saying "you forgot to …" in code review
https://danger.systems/swift/
MIT License
1.05k stars 135 forks source link

Danger-Swift can't get the correct parents of commits #402

Closed el-hoshino closed 3 years ago

el-hoshino commented 3 years ago

Here's a sample PR: https://github.com/el-hoshino/QuickshaRe/pull/27

The Dangerfile can be found in changed files page: https://github.com/el-hoshino/QuickshaRe/pull/27/files

And the commits of this PR can be found in commits page: https://github.com/el-hoshino/QuickshaRe/pull/27/commits

The problem is, obviously there's a merge commit (8c03da41f522864725c545d64fd2c60f9248f1a0) which contains 2 parents in the PR, but Danger-Swift can't find them. All commits' parents are nil.

Not sure if it's related but also Commit 's sha is correct but Danger.Git.Commit 's sha is also nil 🤔

f-meloni commented 3 years ago

Is this data arriving from danger-js? You can use danger pr $PR_LINK --json > file.json to get the json that arrives from danger-js

f-meloni commented 3 years ago

I've downloaded the json from the PR you linked there. The information is not coming into the commit object

"commit": {
            "author": {
              "name": "星野恵瑠",
              "email": "xxx",
              "date": "2021-01-14T05:27:08Z"
            },
            "committer": {
              "name": "星野恵瑠",
              "email": "xxx",
              "date": "2021-01-14T05:27:08Z"
            },
            "message": "Display all messages in Danger",
            "tree": {
              "sha": "eb1ed524d068b1a23e81bdd80cfcad360663e63c",
              "url": "https://api.github.com/repos/el-hoshino/QuickshaRe/git/trees/eb1ed524d068b1a23e81bdd80cfcad360663e63c"
            },
            "url": "https://api.github.com/repos/el-hoshino/QuickshaRe/git/commits/e9ae7efdb3f0fc4898d791f9e6c20bbf8f5fbfab",
            "comment_count": 0,
            "verification": {
              "verified": false,
              "reason": "unsigned",
              "signature": null,
              "payload": null
            }
          },

I think using Git.Commit there is misleading, because it doesn't receive all the properties that Git.Commit has and some of them will remain nil

The information you need can be obtained in two other ways (that we are not currently parsing):

I think is better to parse the first object and change the GitHub commit object to not be the same of Git.Commit but a different object. Will open a PR to fix it

el-hoshino commented 3 years ago

@f-meloni Thanks for the information and glad that this will be fixed in the future! In the meantime is there any way to get the parents information, like parsing the raw json file, from Dangerfile.swift?

f-meloni commented 3 years ago

I've just released the version 3.7.2 with the fix :)

el-hoshino commented 3 years ago

@f-meloni Thanks! Just confirmed parents result with the sample PR! Should I close this issue? or you'll close it at the right time?

f-meloni commented 3 years ago

Cool let me close this