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

Filter out commits by some regular expression #26

Open
rcdailey opened this issue Oct 3, 2023 · 7 comments
Open

Filter out commits by some regular expression #26

rcdailey opened this issue Oct 3, 2023 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@rcdailey
Copy link
Contributor

rcdailey commented Oct 3, 2023

I'd like the ability to specify some filter criteria for the commits that get published as a notification. If we were to use conventional commits, I'd like only feat and fix commits to result in a notification. Other commits (including those in-between, when multiple commits are present) should be ignored (excluded from the notification payload).

One way to provide this would be a way to specify regular expressions that must match the subject line of each commit (I'd expect multiple regular expressions to be OR'd together):

- name: Discord Commits
  uses: Sniddl/[email protected]
  with:
    webhook: ${{ secrets.DISCORD_WEBHOOK }}
    template: simple-link
    include-extras: true
    include-commits:
      - '^fix:'
      - '^feat:'

So if I have three commits in a single push:

fix: I fixed a thing
refactor: I fixed some whitespace
feat: I implemented a new thing

The only commits I'd expect to see in my discord notification are:

  1. fix: I fixed a thing
  2. feat: I implemented a new thing

I can't think of a way to do this outside of the action itself. I suspect this may need to be implemented.

@rcdailey rcdailey changed the title Filter out commits by some prefix Filter out commits by some regular expression Oct 3, 2023
@rcdailey
Copy link
Contributor Author

rcdailey commented Oct 8, 2023

I've implemented this in my fork here. However, the changes I made were significant. I'm not sure if you'd be willing to accept these changes. If so, I'm happy to open a PR.

If not, I will continue to maintain my fork.

@ZebTheWizard
Copy link
Contributor

I like the idea of adding regex and filtering out commits. We could probably make use of regex so people can strip off footers too #25

@ZebTheWizard
Copy link
Contributor

Please make a PR against the 1.7 branch and add testing for the new feature.

please make a copy of the tests/simple directory for each test case.

@ZebTheWizard ZebTheWizard added enhancement New feature or request good first issue Good for newcomers labels Dec 25, 2023
@rcdailey
Copy link
Contributor Author

Thanks for getting back with me. There's a lot of work to clean up the repo IMO, I am happy to start opening smaller PRs to get that out of the way first. For example, I was able to get rid of the node_modules directory; it isn't needed if we use vercel/ncc for distribution.

We can start discussing details in PRs if you'd like.

Please know that I am involved heavily in other projects so I don't have a lot of time to spend here. My issue was opened in October and I only received a response now, 2 months later. I completely respect that you likely had reasons for not being very active, but it will be difficult to come back to this only every couple of months. I think it would help if we can push through my changes with a quicker turnaround. Hopefully that's something you can accommodate.

Thank you for your time!

@rcdailey
Copy link
Contributor Author

I apologize, I should have reviewed your 1.7 branch first. I do see that you removed node_modules in commit ab0d4ac.

There may not be much left to do then. I'll compare my changes with yours and open PRs as needed to get to parity with my fork.

@rcdailey
Copy link
Contributor Author

One other thing I want to recommend. I personally find it tedious to constantly update my actions to use :1.6, :1.7, etc. It requires manual upgrade far too often. Instead, I think :v1, v2, etc are appropriate when there are breaking changes involved. Features should be automatically obtained.

For the purposes of my own needs, I went with v2. You can go with v1 if you feel my features will not introduce breaking changes. Would you be open to changing the way you tag versions so that less manual updates to my workflows is needed?

@ZebTheWizard
Copy link
Contributor

That sounds great to me. I like the idea of keeping PRs small!

If a long list of features is worth adding, I will swap to semantic versioning. Otherwise, I plan to increase the version whenever there is a breaking change or new feature.

You're welcome to open PRs and make suggestions anytime!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants