Add action outputs

[orhun]

Hey! 🐻

I would like to use the output of cargo-semver-checks-action in other steps/jobs as follows:

  semver:
    name: Semver
    runs-on: ubuntu-22.04
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: Check semver
        id: check_semver
        uses: obi1kenobi/cargo-semver-checks-action@v2
        with:
          package: git-cliff-core

      - name: Comment on PR
        if: always() && (steps.check_semver.outputs.error_message != null) && (github.event_name == 'pull_request')
        uses: marocchino/sticky-pull-request-comment@v2
        with:
          header: pr-semver-check-error
          message: |
            Thank you for opening this pull request! ⛰️

            There seems to be semver incompatibility issues reported by [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks).

            Details:

            > ${{ steps.check_semver.outputs.error_message }}

      - name: Update comment on PR
        uses: marocchino/sticky-pull-request-comment@v2
        if: (steps.check_semver.outputs.error_message == null) && (github.event_name == 'pull_request')
        with:
          header: pr-semver-check-error
          delete: true

But it does not set any outputs (i.e. outputs.error_message) so I cannot achieve this.

Inspiration: https://github.com/amannn/action-semantic-pull-request?tab=readme-ov-file#outputs

Can we get this feature in? What do you think?

[obi1kenobi]

Interesting idea! Would love to hear more about the workflow itself.

Is the expectation that users that open PRs containing breaking changes should immediately bump the version number in the Cargo.toml file? Would that be expected to immediately trigger a new release to be created, or can the new version number exist in the repo without being published yet?

[orhun]

Actually I just want to have a sticky comment in the pull request saying that there are some changes that breaks the semver compatibility. Similar to this one: ratatui/ratatui#769 (comment)

I think this makes the error more visible and you don't have to go into the CI logs to figure out what's wrong. I especially want to have this for cargo-semver-checks because it is an important part of my CI. I run it for every pull request and try to avoid semver breakage before creating any releases.

[obi1kenobi]

Absolutely. I want the same thing.

In my mind, this kind of workflow would want to compare the PR branch to the target branch — e.g. pg/new-feature to main, where main is the baseline. Currently, this action only knows how to use the previous published version from crates.io as a baseline, which feels like the wrong choice — it would compare pg/new-feature to v1.2.3 from crates.io. This would would for example repeatedly flag breaking changes that have already been merged in a prior PR but haven't been released yet. This part is a solvable problem, though possibly requires a bit of annoying plumbing.

Also, in addition to the sticky comment, I'd love to have the action flag the specific lines where each issue is happening. This way you can see "this specific function wants more args" as opposed to having to look up the file and function yourself. When I last looked into this, it seemed surprisingly hard to make GitHub comments on the "left side" (the "deleted" side) of a PR. Most linters only comment on the new code so that's what GitHub supports best, but cargo-semver-checks may need to flag e.g. a function deletion in which case there isn't anything on the "additions" side of the PR to comment on. Is this something you've seen any other tool do? Is there a good approach we can use as inspiration?

Bottom line: I'd love to have this workflow work well, and I'd love to work with you on it if you'd be up for that!

[orhun]

this kind of workflow would want to compare the PR branch to the target branch

I agree that this should be the behavior. Currently I'm running this action for every push to the git-cliff repository and I'm getting a failure each time since it is comparing against the crates.io version. Using the main branch as the baseline would solve this problem and I'd get notified about the semver breakage only once.

This part is a solvable problem, though possibly requires a bit of annoying plumbing.

Definitely.

Also, in addition to the sticky comment, I'd love to have the action flag the specific lines where each issue is happening. [...] Is this something you've seen any other tool do? Is there a good approach we can use as inspiration?

Having this would be really nice! I think I've seen similar things but I need to think a bit - can't say a name of a tool off the top of my head now. But it should be doable I reckon.

Bottom line: I'd love to have this workflow work well, and I'd love to work with you on it if you'd be up for that!

I'm up for it! I think I can first tackle #15 - added it to my hacking list :D

What do you think?

[obi1kenobi]

Sounds great! Feel free to ping me anytime if a second pair of eyes could be useful.

There are two classes of problems I was worried about in #15 that would be great to have test cases for:

• I believe the GitHub Action actions/checkout does a "fetch depth 1" clone by default, fetching only the branch it needs and nothing else. That means the local git repo might not know about the target branch's existence. Using the cargo-semver-checks CLI to set a branch might result in an error in this case. It might be necessary to do an explicit git fetch origin <target-branch> first.
• It's possible that the place where the action runs is a fork of the repo in which the PR is being opened. In that case, the target branch is in another repo, and it's possible that upstream repo isn't even added as a remote to the local git repo. This makes the earlier problem even harder: now we might need a git remote add upstream ... command to add the remote, then a git fetch upstream <target-branch> command to learn about the branch, then we need to worry about if the target branch name conflicts with a branch in the origin remote (e.g. a PR from main to main across repos), etc. etc. Lots of edge cases here!

I still believe this is all solvable. But annoying plumbing will definitely be involved too.