Skip to content

feat(ETU-75959): Add support for showing API changes in PR comment - #173

Open
egrimstad wants to merge 3 commits into
mainfrom
feat/ETU-75959_pr-comment
Open

egrimstad wants to merge 3 commits into
mainfrom
feat/ETU-75959_pr-comment

Conversation

@egrimstad

@egrimstad egrimstad commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Legg til kommentar i PR-timeline med "lesbar" liste over endringer i API-specen. Kan styre hvilke endringer man vil se med input show_changes (all/breaking/none). Gjerne sjekk om oppdateringene i README-filene faktisk er forståelige.

Eksempel fra api-spec-registry. Med show_changes='breaking'
image

Med show_changes='all'
image

@egrimstad egrimstad changed the title ETU-75959: Create PR comment with changes feat(ETU-75959): Create PR comment with changes Sep 18, 2026
@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch 2 times, most recently from f3a900b to a59e0a5 Compare September 18, 2026 09:32
Comment thread .github/workflows/publish.yml Fixed
@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch 26 times, most recently from 8075fec to eae0ae1 Compare September 21, 2026 15:22
@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch 4 times, most recently from 10b2eb4 to 5ad2f8b Compare September 25, 2026 08:36
@egrimstad
egrimstad marked this pull request as ready for review September 25, 2026 08:37
@egrimstad
egrimstad requested a review from a team as a code owner September 25, 2026 08:37
@ruiyu2000

Copy link
Copy Markdown
Contributor

Why dont we use https://github.com/actions/github-script instead?

@egrimstad

Copy link
Copy Markdown
Contributor Author

Why dont we use https://github.com/actions/github-script instead?

That might be possible, I can try

@egrimstad

Copy link
Copy Markdown
Contributor Author

Why dont we use https://github.com/actions/github-script instead?

Bleh, I think it would be kinda hacky. Since it is a reusable workflow, I don't think we can just import files from this repo like this: https://github.com/actions/github-script#run-a-separate-file So we would have to check out gha-api to get the files. Having everything be prebuilt in an action seems smoother. But I am open for a suggestion, if there is something i have missed.

I don't want the entire script to be inline, since then we don't get any type checking / autocomplete / syntax highlighting / modules.

Comment thread .github/README.md Outdated
```

Lint and publish the spec to the developer portal in the CD workflow:
Lint, publish, and release the spec to the developer portal in the CD workflow.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking this might be too much info for this README, refer to README-publish instead for these details?

I realize that the golden path include these details though ...
In that case, maybe have some short explanation of difference between "publish" and "relase" and link to README-publish?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was my reasoning. I can't explain why one would use publish.yml with release: false unless I also explain why you would use validate.yml. Since it documents how the different workflows work together, it makes sense to have it here. But yeah.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it is pretty short already.

Comment thread README-publish.md Outdated
artifact: myArtifactName
```

## Publish without releasing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a practical example would be useful here, for users to understand what the use case is.

Comment thread README-validate.md Outdated
@ruiyu2000

Copy link
Copy Markdown
Contributor

Bleh, I think it would be kinda hacky. Since it is a reusable workflow, I don't think we can just import files from this repo like this: https://github.com/actions/github-script#run-a-separate-file So we would have to check out gha-api to get the files. Having everything be prebuilt in an action seems smoother. But I am open for a suggestion, if there is something i have missed.

You are already using main: 'dist/index.cjs', surely gha already checks out this whole repo. Please test importing files with https://github.com/actions/github-script#run-a-separate-file
Also, I think we might as well use https://nodejs.org/learn/typescript/run-natively and skip build entirely

I don't want the entire script to be inline, since then we don't get any type checking / autocomplete / syntax highlighting / modules.

We definitely dont want the whole thing inline.

@egrimstad

egrimstad commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Bleh, I think it would be kinda hacky. Since it is a reusable workflow, I don't think we can just import files from this repo like this: https://github.com/actions/github-script#run-a-separate-file So we would have to check out gha-api to get the files. Having everything be prebuilt in an action seems smoother. But I am open for a suggestion, if there is something i have missed.

You are already using main: 'dist/index.cjs', surely gha already checks out this whole repo. Please test importing files with https://github.com/actions/github-script#run-a-separate-file Also, I think we might as well use https://nodejs.org/learn/typescript/run-natively and skip build entirely

I don't want the entire script to be inline, since then we don't get any type checking / autocomplete / syntax highlighting / modules.

We definitely dont want the whole thing inline.

That's different. The action is referred to using the special syntax uses: $/.github/actions/evaluate-validation which fetches it (and all related code), from the same repo and ref as the reusable workflow is defined in. I mean, I can try 🤷

Additionally, I don't want to skip build entirely, since then we have to run npm ci every time someone runs our script. Seems much more efficient to do all that stuff whenever we make changes, instead of every time.

But it does work! We need to do an extra checkout of gha-api, like this:

      - uses: actions/setup-node@v7
        with:
          node-version: 24
      - name: Checkout gha-api
        uses: actions/checkout@v7
        with:
          path: gha-api
          repository: '${{ job.workflow_repository }}'
          ref: ${{ job.workflow_sha }}
          sparse-checkout-cone-mode: false
          sparse-checkout: |
            foo
      - name: Run script
        uses: actions/github-script@v9
        with:
          script: |
            const { default: main } = await import('${{ github.workspace }}/gha-api/foo/main.ts')

            await main()
            

if the script is in a directory here called foo/main.ts. A run here: https://github.com/entur/api-spec-registry/actions/runs/36110771742/job/108888839000?pr=283

I don't really see how this is much better than referring to an action, though.

@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch from 041c195 to 2595af2 Compare September 28, 2026 10:32
@ruiyu2000

Copy link
Copy Markdown
Contributor

I suppose https://github.com/actions/github-script could remove the current runtime dependencies, but theres no guarantee that we wont need other dependencies in the future.

@egrimstad

Copy link
Copy Markdown
Contributor Author

I suppose https://github.com/actions/github-script could remove the current runtime dependencies, but theres no guarantee that we wont need other dependencies in the future.

Yeah, except for lodash. I guess I could survive without that though.

@egrimstad egrimstad changed the title feat(ETU-75959): Add support for show API changes in PR comment feat(ETU-75959): Add support for showing API changes in PR comment Sep 29, 2026
@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch 9 times, most recently from 9c185d2 to 1182988 Compare October 5, 2026 12:43
@egrimstad
egrimstad force-pushed the feat/ETU-75959_pr-comment branch from 1182988 to 81aef65 Compare October 5, 2026 12:45
@egrimstad
egrimstad requested a review from rikard-swahn October 5, 2026 14:04

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

4 participants