Changeset includes minimum files of certain regex

[vsingal-p]

I am trying to implement a check in which I want that if a change is made in certain files, then there should be more than X number of files.

- when: pull_request.*, pull_request_review.*
    name: 'Some PR check'
    validate:
      - do: or
        validate:
          - do: changeset
            must_exclude:
              regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
          - do: changeset
            must_include:
              regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
            min:
              count: 3

But the problem is, min includes all the changed files in the PR, and not only the ones matching the regex. How to achieve that?

[vsingal-p]

@shine2lay

[vsingal-p]

Or maybe add all to must_include in changeset. I see that must_include option processor supports functionality of all already. It's just that input setting of changeset does not expose that.

[shine2lay]

hey @vsingal-p that may work, i am not quite sure, another solution i can think off the top of my head is to add limit option to changeset which will limit which file are to be considered similar to approvals.

[vsingal-p]

- when: pull_request.*, pull_request_review.*
    name: 'Some PR check'
    validate:
      - do: or
        validate:
          - do: changeset
            must_exclude:
              regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
          - do: changeset
            and:
              - must_include:
                  regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
              - must_exclude:
                  regex: '^(src/main/resources/views/([0-9A-Za-z]+/?)+).*'
              - min:
                  count: 2

I tried this. So basically pass when no file under a specific directory (or subdirectory) is changed.
Or if a file has been changed (must_include)..then nothing outside the package should match (must_exclude)..and the changeset should be greater than 2 files

[vsingal-p]

Using limit might get redundant I guess as we would be copy pasting same regex in most cases I can think of

What say, let's give all a shot? I can do that in my forked application and get back to you whether it works or not

[vsingal-p]

Also I want to perform an action when the second changeset option in or is fulfilled. Is that possible?
Current doc says that we can have actions on overall success of the check, not it's individual constituents

[shine2lay]

Using limit might get redundant I guess as we would be copy pasting same regex in most cases I can think of

What say, let's give all a shot? I can do that in my forked application and get back to you whether it works or not

How would all work? can you give a sample config using all?

Also I want to perform an action when the second changeset option in or is fulfilled. Is that possible? Current doc says that we can have actions on overall success of the check, not it's individual constituents

For now, i think you can have a second recipe that only include the individual constituents you want and have an action for the second recipe.

[vsingal-p]

- when: pull_request.*, pull_request_review.*
    name: 'Some PR check'
    validate:
      - do: or
        validate:
          - do: changeset
            must_exclude:
              regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
          - do: changeset
            must_include:
              regex: 'src/main/resources/views/([0-9A-Za-z]+/?)+'
              all: true

This would mean if any change is made in any file under views package, then all the files should be from that package only

[vsingal-p]

For now, i think you can have a second recipe that only include the individual constituents you want and have an action for the second recipe.

But that would show up in my PR checks, no? Is their any way we can prevent that?

[vsingal-p]

@shine2lay I saw some weird behaviour. When I added all to must_include under changeset..it worked. Out of the box.
Despite the fact that supportedSettings of changeset validator does not support must_include.all

I saw that we store each configurable option in validator as option and each validator is a combination of multiple option . So why do we have option definition in supportedSettings in validator ?
A validator should define supported options, and the definition of an option and it's validation should be it's responsibility.
I see some exceptions to this..like required in approvals. But do you think the restructuring is apt and worth it?

[shine2lay]

For now, i think you can have a second recipe that only include the individual constituents you want and have an action for the second recipe.

But that would show up in my PR checks, no? Is their any way we can prevent that?

Yes we can prevent a second check from appearing, Mergeable will only add check action if no other action is specified. So if you specify an non check action for your recipe, check will no longer appear by default

@shine2lay I saw some weird behaviour. When I added all to must_include under changeset..it worked. Out of the box. Despite the fact that supportedSettings of changeset validator does not support must_include.all

I saw that we store each configurable option in validator as option and each validator is a combination of multiple option . So why do we have option definition in supportedSettings in validator ?

Yes, there are some settings that is not specified in the supportedSettings that should work out of the box. We designed to implement features in a way that is easily reusable, that being said, sometimes, it doesn't work out of the box because maybe the validator will need to do a few extra step of pre-processing because passing it into the options processor. Another reason why we haven't add it to all yet is because there are no tests for them yet.

I would love to hear if you have a different solution.

A validator should define supported options, and the definition of an option and it's validation should be it's responsibility. I see some exceptions to this..like required in approvals. But do you think the restructuring is apt and worth it?

If you are suggesting that all validator must support a basic subset of settings, I am not sure if that's the best idea since we imagine that validator could be anything in the future (i.e 3rd party API calls like JIRA) and not just confined to Github related items. WDYT?

[taylor-cedar]

Hey everyone. I am having the same issue and just came across this. It does not appear that all is supported out of the box
Error : UnSupportedSettingError: validator/changeset: must_exclude.all option is not supported

Would you support a PR to add this option?

[shine2lay]

@taylor-cedar yes, would definitely support it. Please tag me if you open a PR