mikepenz/release-changelog-builder-action

Clarify documentation for testing local

paul-dingemans opened this issue · 3 comments

paul-dingemans commented

In pinterest/ktlint#2203 you suggested to use codespace and local testing. I stumbled upon some problems as documentation is not entirely clear for user that are not yet familar with some (basic) concepts.

  • Codespaces are not explicitly meant in documentation. But if you do, you might want to mention that the Codespace has to be created on mikepenz/release-changelog-builder-action and not on the repo on which you want to test the plugin.
  • For the readonly github token it would be nice to clarify how it should be created. After some googling, I found out that I just needed to create a new token without specifying any additional rights.
  • After the export of the token, you mention to run npm test -- custom.test.ts. But it is much more clear to first mention that this file needs to be created first. And of course, that you need to change the owner and repo name in the example and some of the other settings. Finally, the example is missing a mandatory parameter (cache).

For testing purposes of a custom configuration, the usage of jest test feels a bit weird as I only want to see the created output. If you don know what jest is, you might not realized that the error you see on the console is from the jest test.

mikepenz commented

@paul-dingemans thank you very much for the detailed feedback. Really appreciate this, and I agree that this part needs to be further improved.

Will take all your proposals and feedback into account and make iterations for it.

More than happy to take any PRs on improving the docs too, if you have additional ideas how to make it better for users.

paul-dingemans commented

One other problem that I encountered is unclarity about whether to use ${{...}} or #{{..}}in the jsonConfiguration object. When using the first notation in a codespace in aconfiguration.jsonfile it worked well. When using thejsonConfigurationin aworkflow.yml` file it resulted in a failing pipeline job:

[Invalid workflow file: .github/workflows/generate-changelog.yml#L32](https://github.com/pinterest/ktlint/actions/runs/5984248143/workflow)
The workflow is not valid. .github/workflows/generate-changelog.yml (Line: 32, Col: 30): Unrecognized named-value: 'TITLE'. Located at position 1 within expression: TITLE

The #{{...}} notation seems to work in both.

  • 👍1
mikepenz commented

@paul-dingemans that's a good point. The main reason both variants exist is that I started off with only a json file configuration option, and there I coud use ${{ }} however, with v3 at I introduced the configurationJson as an additional solution to configure. Unfortunately ${{ }} is already reserved for GitHub actions own syntax, so I had to introduce a new one (e.g. #{{ }})

Now as you say that, I did support both for a while, but I think it's fair to say I should inverse the default (also from docs perspective) to #{{ }} and slowly phase out ${{ }}.

Going to update the docs around that too.

  • 👍1