Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A bunch of unexpected behaviour #605

Closed
darrellwarde opened this issue Dec 1, 2021 · 12 comments
Closed

A bunch of unexpected behaviour #605

darrellwarde opened this issue Dec 1, 2021 · 12 comments
Assignees

Comments

@darrellwarde
Copy link

Sorry for the not very specific title, but having a few issues!

Firstly, the exhaustive setting does not appear to be working as expected. Despite having this enabled for each category (features, fixes, tests) on a category label plus a product label, features are appearing in the fixes list, etc.

Secondly, date ranges between tags don't seem to be resolving properly. Despite finding the right tag range (@neo4j/[email protected]...@neo4j/[email protected]), the date range that it infers from that (2021-11-01T10:03:42.000Z to 2021-12-01T13:39:00.000Z), as the previous tag was made 22 days ago, so just over a week over what it's some up with as the start date.

Any ideas on the above? Thanks for any help!

Full log below:

Run mikepenz/release-changelog-builder-action@v1
  with:
    configuration: config/release-changelog-builder-action/configuration-graphql.json
    toTag: @neo4j/[email protected]
    ignorePreReleases: false
    failOnError: false
    commitMode: false
  env:
    GITHUB_TOKEN: ***

📘 Reading input values
  
  
  
🔖 Resolve previous tag
  ℹ️ Found 57 (fetching max: 200) tags from the GitHub API for neo4j/graphql
🚀 Load pull requests
  ℹ️ Comparing neo4j/graphql - '@neo4j/[email protected]...@neo4j/[email protected]'
  ℹ️ Found 271 commits from the GitHub API for neo4j/graphql
  ℹ️ Fetching PRs between dates 2021-11-01T10:03:42.000Z to 2021-12-01T13:39:00.000Z for neo4j/graphql
  ℹ️ Retrieved 173 merged PRs for neo4j/graphql
  ℹ️ Retrieved 271 release commits for neo4j/graphql
📦 Build changelog
  ℹ️ Sorted all pull requests ascending: ASC
  ℹ️ Used 1 transformers to adjust message
  ✒️ Wrote messages for 24 pull requests
  ℹ️ Ordered all pull requests into 3 categories
  ✒️ Wrote 18 categorized pull requests down
  ✒️ Wrote 6 non categorized pull requests down
  ✒️ Wrote 0 ignored pull requests down
  ℹ️ Filled template

Configuration below:

{
    "categories": [
        {
            "title": "## 🚀 Features",
            "labels": ["feature", "graphql"],
            "exhaustive": true
        },
        {
            "title": "## 🐛 Fixes",
            "labels": ["fix", "graphql"],
            "exhaustive": true
        },
        {
            "title": "## 🧪 Tests",
            "labels": ["test", "graphql"],
            "exhaustive": true
        }
    ],
    "tag_resolver": {
        "method": "sort"
    }
}
@mikepenz mikepenz self-assigned this Dec 1, 2021
@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

Hi there @darrellwarde

Thank you for the report. I'll have a look at this.

Was this the current specification in your github actions? neo4j/graphql@5020318#diff-d44dd879a02a07f5781424a643498042e49b0b3d49d2682d176d7322e89f69a6R384-R391

@darrellwarde
Copy link
Author

Hi there @darrellwarde

Thank you for the report. I'll have a look at this.

Was this the current specification in your github actions? neo4j/graphql@5020318#diff-d44dd879a02a07f5781424a643498042e49b0b3d49d2682d176d7322e89f69a6R384-R391

Apologies, I should have given you that as well to start with!

That's from a different workflow file, but identical YAML, the actual block is https://github.com/neo4j/graphql/blob/master/.github/workflows/release.yml#L518-L525

Thanks for the speedy response!

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

Thank you @darrellwarde

For the second question you asked, regarding having a different fromDate than the fromTag. This results from the retrieval of the commits between fromTag to toTag:

https://github.com/mikepenz/release-changelog-builder-action/blob/develop/src/commits.ts#L44-L49

And branches which ended up in the toTag may have been active before the fromTag.

Given the date range the action fetches the PRs which were merged within the range:
https://github.com/mikepenz/release-changelog-builder-action/blob/develop/src/releaseNotes.ts#L109-L115

Which will then be filtered out depending if they were part of the release.

Based on the last time we checked there hasn't been a better API on GitHub yet which would deliver only PRs which were merged between 2 tags.

Hope that answers the second question. Looking now in your other question

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

Oh one thing I saw based on your mono repo setup. The action offers the ability to pre-filter tags or transform tags if they are prefixed like in your case. So you can still use real semver ordering.

This was showcased for example:
d5c72c2#diff-5dd6a20ac6fda7c3a6519cec8213222616372409d92d79bc83c9e8942638d32cR52-R60

And maybe also helpful. It's possible to checkout this repo and run parts of the action locally. So you don't have to test it within the repo. This even comes with debugging support:
#576 (comment)

I am currently using this branch: https://github.com/mikepenz/release-changelog-builder-action/tree/feature/605_test
And run the test via:

npm test -- neo4j.test.ts

For the local action to run you need to give it a token so the gitHub API can be called

# GitHub token for release builder
export GITHUB_TOKEN=your_read_only_github_token

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

Do you possibly have an example of which PR ended up in the wrong category?

Based on the below output it seems to at least make sense

    ## 🚀 Features
    
    - Feat/fulltext
       - PR: #563
    - Sorting by relationship properties for union and interface fields
       - PR: #598
    - Feat/Aggregate Sum
       - PR: #597
    - Feature/connectOrCreate
       - PR: #575
    
    ## 🐛 Fixes
    
    - Fixed edge updates on one to one relationships
       - PR: #579
    - fix/559: add subscription type fields to resolve tree field resolver
       - PR: #576
    - Querying DateTime in the edge of a connection throws an error
       - PR: #587
    - Generate scalar typedefs before extra definitions
       - PR: #599
    - Fix/564: Async error when reading from JWKS endpoint
       - PR: #578
    - Fix for missing auth parameter
       - PR: #603
    - Fix: Project All Interface Fields For Types Implementing `@relationship` Interface
       - PR: #590
    - Fix: Escape all @node directive labels
       - PR: #617

@darrellwarde
Copy link
Author

darrellwarde commented Dec 1, 2021

Strange, our real output was very different! Luckily we haven't fixed the changelog yet, but it ended up like https://github.com/neo4j/graphql/releases/tag/%40neo4j%2Fgraphql%402.5.0

You can see that almost every, if not every, PR is in each category.

Just in case we edit it shortly
## 🚀 Features

🐛 Fixes

🧪 Tests

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

@darrellwarde oh, I should have looked at that earlier. It looks you are using the v1 of the action:

mikepenz/release-changelog-builder-action@v1

https://github.com/neo4j/graphql/blob/master/.github/workflows/release.yml#L520

If you want to use v1, this was the latest v1 README and state for the action:
https://github.com/mikepenz/release-changelog-builder-action/tree/v1.8.3

Exhaustive did not exist back then, as such the graphql label will match for every category.


If you would like to use the newer flags please update to v2.

You may want to even use more specific versions like specifying v2.7.0 so updates of the actions won't affect you, if things are tested and working.

And as usual the security warning: If you want to be super careful only use the GIT-SHA of a release so no altering at all will be possible (not as big of a concern for an open source repo, but always like to add this note :D)

@darrellwarde
Copy link
Author

Well now I feel like an absolute numpty! Thank you for spotting that, you've probably saved me hours of scratching my head over this.

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

No worries :D I didn't look at it directly either.

Just edited my message above with some additional details.


But yeah if you want to try and modify the action and configurations, it's much faster and easier if you run it locally :). So you can pre-test things without needing to tag on the repo, and wait until things run through.

@darrellwarde
Copy link
Author

This was really great support, thank you! Pulled down that branch you pointed me to and used it to generate the correct changelogs to copy and paste in, saved me a load of time!

Now bumped the version to v2 and doing a release of a different product to validate. Many thanks again! 😄

@mikepenz
Copy link
Owner

mikepenz commented Dec 1, 2021

@darrellwarde you are very welcome. Happy to be able to help :).

Also really nice actions setup you have there. If there's anything we can do to improve the action, or clarify things, .. let me know.

Have a great day!

@darrellwarde
Copy link
Author

Thanks for the kind words, you have a great day too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants