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

Add keyword trigger condition #3053

Closed
wants to merge 1 commit into from

Conversation

kdumontnu
Copy link

@kdumontnu kdumontnu commented Jan 9, 2021

This enables a new trigger for keywords in the commit message or title. The usage is as follows:

trigger:
  keyword:
  - "*[RUN PIPELINE]*"

Should only run if the keyword is matched.

or

trigger:
  keyword:
    exclude:
    - "*[SKIP DEPLOY]*"

Should not run if the keyword is matched.

I considered simplifying the notation by appending * to each end of the keyword strings, but it would give the user less control on matching specific strings.

Note: The way this is currently implemented follows the same syntax as other triggers, but requires adding the Keyword condition to drone-yaml. However, I see that the GitHub repository is currently archived, so I'm unable to open a PR.

On further inspection it looks like drone-yaml Conditions essentially expects each argument to be a "path" in the matching operation. Passing an arbitrary string (message or title) will probably result in odd behavior. The solution here appears to be to simply create a separate Match function for Condition that uses go strings.Contains and remove the wildcard operators above. Again, this is in drone-yaml, which appears to be private.

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kdumontnu
Copy link
Author

Just saw this issue, which appears closely related to this. Seems like there was no consensus on the proper syntax, so I propose to leave it up to the user which strings to match.

@tphoney
Copy link
Contributor

tphoney commented Jul 1, 2021

Hey @kdumontnu this seems to have went a little stale, there are a few things i would like to raise. It is unclear how this would work and it needs a little bit more explanation. I would suggest creating a proposal for this feature here https://github.com/drone/proposal first . So we can gather feedback on this would work. I am closing the issue for now, due to staleness not signing the CLA and failing tests.

@tphoney tphoney closed this Jul 1, 2021
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

3 participants