CircleCI-Public / path-filtering-orb

MIT License
24 stars 62 forks source link

Only last commit is checked #53

Closed shaharl-sternum closed 1 year ago

shaharl-sternum commented 2 years ago

Orb version: 0.1.3

What happened:

If multiple commits are being pushed, only the last one is checked in the filtering path

Expected behavior:

All the commits will be checked since the last push and run

Additional Information:

kelvintaywl commented 2 years ago

hi @shaharl-sternum , thank you for reporting this with you.

To help us debug further, would you mind sharing your config.yml here with us (in particular, the section where you are using the path-filtering Orb)?

This will help us understand your context better!

For sharing, the current path-filtering/filter job definition uses a base-revision parameter (which defaults to main branch). This parameter is used as the base to compare commits against, to figure out which files and folders have changed.

We can also see the comparison logic here: https://github.com/CircleCI-Public/path-filtering-orb/blob/9b229fa9b2b3974000a8ed53814dab609128b745/src/scripts/create-parameters.py#L20-L49

sagimonza commented 2 years ago

Hi, suffering from the exact same problem. My config.yml:

workflows:
  run-filter:
    jobs:
      - path-filtering/filter:
          base-revision: main
          mapping: |
            apps/app1/.* run-app1 true
            apps/app2/.* run-app2 true
            libs/.* run-apps true
            package.json run-apps true
            package-lock.json run-apps true
            Dockerfile run-apps true
          config-path: ".circleci/dynamic_config.yml"
          circleci_domain: "cci.domain.com"
kelvintaywl commented 2 years ago

hi @sagimonza ,

Thank you for your updates. I believe the "only last commit is checked" happens when you are pushing multiple commits to your base-revision branch then (main in your case).

https://github.com/CircleCI-Public/path-filtering-orb/blob/9b229fa9b2b3974000a8ed53814dab609128b745/src/scripts/create-parameters.py#L33-L42

Is this an issue for you when a make a PR or merge a PR?

sagimonza commented 2 years ago

@kelvintaywl thanks for assisting. It's a PR merge issue - as we usually merge a PR with Rebase and without Squashing, it means that a single PR having multiple commits merged to main will only trigger pipelines based on the latest commit in the PR.

kelvintaywl commented 2 years ago

hi @sagimonza

Thank you for the details, and apologies for the delayed follow-up here!

I am guessing this may not be an issue for your team if you choose to squash the PR when merging then (since the PR will become 1 commit only). However, I am also aware this may not be ideal for certain teams.

I understand you have filed a Support ticket with us earlier, so we will reach out on the ticket re: updates then. We are still awaiting our Engineering team for updates.

In the meantime, for others seeing the same issue, please let us know if the Squash and Merge option when merging a GitHub PR is a good-enough workaround for your team? 🙇

samkahchiin commented 1 year ago

@kelvintaywl It is not an ideal case for our team. When we want to deploy to production, we usually merge/rebase multiple commits from the staging branch to production. We would not like to squash the commit in this case because we will lose the precious commit message/changes (1 commit = 1 story). So it would be great if the team can provide us the options to compare the changes before/after the push VS the latest commit.

h42zhu commented 1 year ago

@samkahchiin The path-filtering/filter job allows specifying the base-revision parameter. So in this case, adding

- path-filtering/filter:
          base-revision: << pipeline.git.base_revision >>
          mapping: |
            XXX XX XX
          config-path: ".circleci/something.yml"

may work for your case?

Fernando-Abreu commented 1 year ago

I'm going to close this issue since we haven't heard back from the user