PR is not exists on the release notes

[wellDan28]

Hey,
I've facing some unexpected behavior:
In my repo the tag v1.13.0 was added after a pull-request was merge to the master but the pull-request is not include in the release notes.
I'm running the following command:
gren r --token="XXXXXXXXXXX" --data-source=prs --tags="v1.13.0..v1.10.0"

Demo repo:
https://github.com/wellDan28/gren-test/releases

Is this is the expected behavior in this scenario?

[alexcanessa]

Hello @wellDan28

Let me investigate and I'll come back to you.

[svanzoest]

I am seeing this as well, where the last commit does not seem to be accounted for.

[phun-ky]

I am also seeing this, the commits from a merged PR is ommited. Is it because of fast forward or something? I see the desired commits with git log though..

[alexcanessa]

Ok, looks like this is a top priority.

Working on this next 👍

[lukemartin]

I've been looking into fixing this, and have a hit a bit of a roadblock.

I think the main problem is down to the time difference between a pull request being closed and the actual merge commit created.

For example:

$ git log

commit 754bebe07899617d4b4525bfa9b8596b4e9c8148 (HEAD -> master, tag: v1, origin/master, origin/HEAD)
Author: Luke Martin <luke@lukemartin.me>
Date:   Fri Mar 2 22:48:59 2018 +0000

	Update README.md (#2)

commit 85fbf8806afc57d245f5bf7f1b923e14f3c3e166
Author: Luke Martin <luke@lukemartin.me>
Date:   Fri Mar 2 22:48:28 2018 +0000

	Update README.md (#1)

When creating a release:

gren release --data-source=prs

The output only contains details for PR #1

Here we can see the merge commit 754bebe was created at 22:48:59. But, when inspecting the metadata for the missing PR:

{
	...
	id: 301929932,
	number: 2,
	title: "pr2",
	state: "closed",
	created_at: "2018-03-02T22:48:54Z",
	updated_at: "2018-03-02T23:10:46Z",
	closed_at: "2018-03-02T22:49:00Z",
	body: "pr2",
	...
}

The PR is actually marked as closed 1 second later, at 22:49:00. Therefore gren thinks the PR was closed after the tag was created, but we know it wasn't.

We can't even use a combination of state and updated_at because post-merge comments will update the value making it unreliable.

I've poked around the GitHub API docs and I can't find anything that might help here. @alexcanessa any ideas? I'm happy to attempt a fix if you can push me in the right direction.

Cheers! :)

[lukemartin]

I've been thinking about this a bit more, and was wondering if using commit id's instead of dates might be more reliable for this sort of thing?

exxxxxxx (tag3)
dxxxxxxx
cxxxxxxx
bxxxxxxx (tag2)
axxxxxxx (tag1)

// release tag3
// commits in release must be `cxxxxxxx,dxxxxxxx,exxxxxxx`
// look for all closed PRs which have a merge commit id which is one of `cxxxxxxx,dxxxxxxx,exxxxxxx`
// we can get this from `listIssueEvents`

[wellDan28]

I'm thinking that adding a second to each of PR merged time fields might be a sufficient solution (not clean but sufficient)

[wellDan28]

@alexcanessa what do you think about 👆 ?

[alexcanessa]

@wellDan28 not totally sure about the "adding a second" solution as we don't know if there is a set time or a delay caused by their server etc.
In fact, the data is different for the last to PRs merged in this repo.

@lukemartin looking for all commits is quite an expensive solution in a real scenario when you have lots of commits.

Possible solution

Although there are a few properties that contain data that we might use.
Looking at two PRs for this repo:

Pull Request #157

{
    "number": 157,
    "created_at": "2018-04-03T05:47:03Z",
    "updated_at": "2018-04-03T08:43:57Z",
    "closed_at": "2018-04-03T08:43:57Z",
    "merged_at": "2018-04-03T08:43:56Z",
    "merge_commit_sha": "78786b31a7a85afc884092af41c681669fe9652e"
}

Pull Request #148

{
    "number": 148,
    "created_at": "2018-03-08T11:00:44Z",
    "updated_at": "2018-04-19T09:56:18Z",
    "closed_at": "2018-04-19T09:54:59Z",
    "merged_at": "2018-04-19T09:54:59Z",
    "merge_commit_sha": "9ff8e2a191ff5ed3b900749f7a3a059f240ad646",
}

I can see that:

• In #157: closed_at is one second after merged_at
• In #148: closed_at is the same time as merged_at

Maybe one of the two properties (closed_at I would assume) is what we're actually looking for.

The commit SHA

In case this isn't enough, there is a merge_commit_sha, that might give us the actual time.

This one is the commit for #148:

{
  "sha": "9ff8e2a191ff5ed3b900749f7a3a059f240ad646",
  "commit": {
    "author": {
      "name": "Alexander Vassbotn Røyne-Helgesen",
      "email": "XXXXXXXXXXXXXXXXX",
      "date": "2018-04-19T09:54:58Z"
    }
}

Now, looks like commit.date ("2018-04-19T09:54:58Z") is even one second less than both merged_at and closed_at. (which makes me think that the commit solution won't work)

---

I think we should look into those properties and see what is a right reflection in terms of timing.

[shaikatz]

Hi any news about that?
I've just experienced it in Kamus