Bug: commenting on PRs from the previous release

[zeke]

Example: electron/i18n#280 (comment)

I think the problem here may be that we're including commits from the previous release when searching:

release-notifier/index.js

Lines 48 to 53 in d817aed

	const {data: commits} = await context.github.repos.getCommits({
	owner,
	repo,
	sha: currentRelease.tag_name,
	since: previousRelease.created_at
	})

@gr2m does that sound right? Maybe should be searching for previousRelease.published_at instead of previousRelease.created_at?

[zeke]

Thinking about this more, published_at vs created_at shouldn't matter...

[gr2m]

that part looks good, we do the same in semantic-release:
https://github.com/release-notifier/release-notifier/blob/master/index.js#L48-L53

[gr2m]

I really wanna move this into an octokit plugin that then can be used by both semantic-release and release-notifier, can’t wait to get to it

[pvdlg]

For information we found other cases of false positive. In the case described in semantic-release/semantic-release#726 there was a comment on a PR that referenced some code. The search API when passed a commit sha would return:

• The PR containing the commit
• But also the commit that added the code referenced in the comment on the first PR

So the search would return 2 results, the expected PR and one seemingly unrelated.

I implemented a fix in semantic-release/github#59.

In addition I added more filtering:

• Search for only merged PRs
• Comment only on closed PRs/issues as PR could Fix an issue with keyword, but the issue might have been re-opened if it turns out it's not really fixed

[pvdlg]

that part looks good, we do the same in semantic-release:
https://github.com/release-notifier/release-notifier/blob/master/index.js#L48-L53

We don't do the same thing in semantic-release. Here it seems you use are finding the last GitHub releases and find the commits based on the date of the previous release with since.

In semantic-release we use only tags to find the new commits since the last release.
We use git log <last-version-tag>..HEAD.

There is cases in Git in which the order of the commits in the history is not chronological. So I'm not sure using the commit date would be consistent.