upsidr / merge-gatekeeper

Get better merge control
MIT License
85 stars 14 forks source link

Support spaces and newlines in ignored jobs #43

Closed Jrc356 closed 1 year ago

Jrc356 commented 1 year ago

Similar to #39, this PR removes trailling and leading whitespace from ignored job. It also adds support for using newlines in the ignored jobs. This should allow users to use merge gatekeeper in the following way:

jobs:
  merge-gatekeeper:
    runs-on: ubuntu-latest
    steps:
      - name: Run Merge Gatekeeper
        uses: upsidr/merge-gatekeeper@v1
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          ignored: |
            my_job1,
            my_job2,
            my_job3
rytswd commented 1 year ago

Hi @Jrc356, thanks for raising this, and it's a great idea to support this use case! 🥰 I have to admit that we have lost track of #39, so will make sure to get it reviewed and merged this week. If you prefer, we could include this change as a part of that ticket - let us know how you want to proceed 👍

Jrc356 commented 1 year ago

Hi @Jrc356, thanks for raising this, and it's a great idea to support this use case! 🥰 I have to admit that we have lost track of #39, so will make sure to get it reviewed and merged this week. If you prefer, we could include this change as a part of that ticket - let us know how you want to proceed 👍

@rytswd I do not mind either way honestly. They are pretty similar in nature but #39 included some other things in it. It also doesn't add support for new lines. If you guys want to merge into one PR, I'm perfectly ok with that

rytswd commented 1 year ago

@Jrc356 Sorry for getting back to you late, thanks for your input! I believe the merge of #39 improved the situation a lot for this use case, but I also created a follow-up PR #45 to refine implementation. Once that's merged and released, we should have this use case of multiline string input fully supported 👍

Jrc356 commented 1 year ago

@Jrc356 Sorry for getting back to you late, thanks for your input! I believe the merge of #39 improved the situation a lot for this use case, but I also created a follow-up PR #45 to refine implementation. Once that's merged and released, we should have this use case of multiline string input fully supported 👍

@rytswd sounds good to me, should I close this then?

rytswd commented 1 year ago

@Jrc356 Yes, that would be appreciated! I will create a separate Issue to keep track of this request instead 👍