Simek / yarn-lock-changes

Creates a comment inside Pull Request with the human-readable summary of changes to the Yarn lock file. Works with every Yarn version (classic and berry).
https://github.com/marketplace/actions/yarn-lock-changes
MIT License
132 stars 20 forks source link

Feature request: support multiple yarn.lock files #29

Closed elektronik2k5 closed 3 years ago

elektronik2k5 commented 3 years ago

Hello and thanks for developing and maintaining this action!

I have a bit of an unusual setup, in which my repo contains several projects. Sort of like a monorepo, but without all the extra tooling. Basically, my repo is in transition from a legacy app to a modern one:

/package.json and /yarn.lock for shared devDependencies. /legacy-app/package.json and /legacy-app/yarn.lock for the legacy app. /modern-app/package.json and /modern-app/yarn.lock for the modern app.

Is there a chance to add support for either detecting multiple yarn.lock files - or - specifying them explicitly, just like the current path allows, but for more than one?

Simek commented 3 years ago

Hi @elektronik2k5, thank you for the kind words. 🙂

At this moment, to perform check on many locks you can just use the action multiple times:

name: Yarn Lock Changes
on: [pull_request]

jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v2
      - name: Yarn Lock Changes Main
        uses: Simek/yarn-lock-changes@main
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
      - name: Yarn Lock Changes Legacy App
        uses: Simek/yarn-lock-changes@main
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          path: legacy-app/yarn.lock

You can also use the dorny/paths-filter action to run the lock checks conditionally, if you want to.

Alternatively, you can create separate workflow files for each of the apps, but it might be an overkill, if you do not have the separate workflows for each app already implemented in.

Please let me know, if at least one of the suggestions is feasible in your case. If not I will look at possibility to support array inputs in path. But generally I would like to avoid including more than one lock changes summary per comment.

Example

elektronik2k5 commented 3 years ago

@Simek thanks so much for the swift and detailed response!

I tried your suggestion of multiple steps calling the action with different paths and names. It works but unless I also set updateComment: false, the same comment gets overridden (edited) by the latter actions, which makes results in having only the very last one visible.

The problem with updateComment: false is that it spams the PR comments with the number of action instances I've defined - per push. The result is... far from optimal:

image

Manually deleting superseded comments helps a bit, but is really tedious and unsustainable. :(

Is there a way to associate each action name (which are unique in my case) with a single comment to update? Similarly to how updateComment updates the single global comment per PR, only per action, per PR.

Simek commented 3 years ago

Thank you for the comprehensive feedback! 🙂

I tried your suggestion of multiple steps calling the action with different paths and names. It works but unless I also set updateComment: false, the same comment gets overridden (edited) by the latter actions, which makes results in having only the very last one visible.

Great catch, it's definitely a bug. I will update the action, so it can find the correct comment which should solve your issue.

Simek commented 3 years ago

@elektronik2k5 If you are using main as a version the fix should be already accessible in your workflow (if you are not, please switch to this version for a while).

It would be nice if you can confirm that the latest changes have fixed the issue, if so, I will release the new, tagged version to the Marketplace.

elektronik2k5 commented 3 years ago

Thank you! Works great :) Looking forward to the release.