Diff does not distinguish between changes on source and target branch

[kwin]

Describe the bug
A changelog modification is incorrectly detected if it only exists on the target branch.

To Reproduce

1. Create new branch from master: test1
2. Commit a changelog entry to master
3. Create a PR from test1 to master

Expected behavior
The changelog enforcer should fail.

** Actual behavior**
The changelog enforcer does not fail.
This is due to the way how the diff is generated which takes into account changes on both sides (source and target branch).

A link to or sample of your workflow
https://github.com/Adobe-Consulting-Services/acs-aem-commons/runs/3352468125?check_suite_focus=true

[dangoslen]

Hi @kwin! Thanks for reporting.

Interesting. So this is because test1 doesn't have all of the commits from master when it runs. Is that the case?

[kwin]

Exactly, there are some commits on master which are not on the branch from the PR. Pretty common for feature branches which live a bit longer.

[dangoslen]

@kwin I haven't forgotten about this. I simply haven't had the time to dig into the guts of git to figure it out.

If you have ideas, I'm welcome to them!

[dangoslen]

Actually, I believe I have a fix for this. Was simpler than I anticipated.

I'll create a v2.3.2 release to address.

[dangoslen]

I was looking into this and the problem is that the changelog enforcer would need to pull all of the git history (as far as I can tell) to do this diff properly. This might not be the right reason to pull such history.

I need to do some thinking about if there is a way to tell if the full history has been downloaded and then changing the diff command that runs based on that at runtime. I think that is possible, but I am not sure right now. I've closed the PR I had open until I can do some more thinking on that.

[kwin]

Can't you use the command outlined in https://stackoverflow.com/a/29810438 ?

[dangoslen]

Indeed that is the right approach. But as I said, you need a deeper commit history for it to work.

[kwin]

Probably switching from git cli to https://docs.github.com/en/rest/reference/repos#commits is required to make it work on shallow clones.

[dangoslen]

I think you are right. It's doable, but it will be a significant refactor. I'll aim to put this in a v3.0 release.

[dangoslen]

Hi @kwin

I've got an early release candidate that I believe solves this problem. Could you try that workflow using dangoslen/changelog-enforcer@releases/v3.0?

[kwin]

The test release 3.0 does not work. It gives the following error

Run dangoslen/changelog-enforcer@releases/v3.0
Skip Labels: Skip-Changelog,skip-changelog
Changelog Path: CHANGELOG.md
Missing Update Error Message: No update to CHANGELOG.md found!
Expected Latest Version: 
Version Pattern: ^## \[((v|V)?\d*\.\d*\.\d*-?\w*|unreleased|Unreleased|UNRELEASED)\]

Error: files.filter is not a function

(https://github.com/Adobe-Consulting-Services/acs-aem-commons/runs/4237701929?check_suite_focus=true)

[dangoslen]

Ah, yes. I believe you are missing the additional token that is required for this version. Since the new version uses the GitHub API, it needs an access token. You can use the one auto-generated by GHA.

- uses: dangoslen/changelog-enforcer@v3
  with:   
    token: ${{ secrets.GITHUB_TOKEN }}

[kwin]

This won't work with PRs from forked repos. Please rather use the token from the github.token context (https://docs.github.com/en/actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow):

Important: An action can access the GITHUB_TOKEN through the github.token context even if the workflow does not explicitly pass the GITHUB_TOKEN to the action.

[dangoslen]

You can use any token you want. It's just an argument to the action.

[kwin]

I don't think there is a good reason to not take the github.token from the context. Less configuration and should always work. Why do you force user's to configure it?

[dangoslen]

Why do you force user's to configure it?

Maybe it is just the actions I've used, but I've seen many requiring the token to be specified explicitly rather than assumed. It is a good point though.

I'll take a look at using the github.token value as a default if a token isn't supplied.

[kwin]

Not sure whether the GitHub documentation is actually wrong: https://github.community/t/accessing-gh-context-in-actions/206203/3 and you always need to explicitly pass it as parameter...

[dangoslen]

@kwin I've updated that branch to use the github.token as the default

[kwin]

@dangoslen Can I expect the release 3.0 soon (currently there is only the branch, but no tag and the commits are not on master despite the fact that this issue is closed)?