Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support spaces and newlines in ignored jobs #43

Closed
wants to merge 2 commits into from

Conversation

Jrc356
Copy link
Contributor

@Jrc356 Jrc356 commented Oct 21, 2022

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
Copy link
Member

rytswd commented Oct 24, 2022

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
Copy link
Contributor Author

Jrc356 commented Oct 24, 2022

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 rytswd mentioned this pull request Oct 29, 2022
@rytswd
Copy link
Member

rytswd commented Oct 29, 2022

@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
Copy link
Contributor Author

Jrc356 commented Oct 31, 2022

@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
Copy link
Member

rytswd commented Oct 31, 2022

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

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.

None yet

2 participants