feat(ci): Add changed lines check

[desi]

We (MMIG eng) have decided that PRs greater than 1000 lines are too big and need to be broken down, thus this check will be used in metamask-extension, metamask-mobile and core to prevent PRs which are too big.

https://github.com/MetaMask/MetaMask-planning/issues/3739

[HowardBraham]

I think this needs override ability. Some PRs have to be very large. And those should also be added to this list MetaMask/metamask-extension#30661

[desi]

I think this needs override ability. Some PRs have to be very large. And those should also be added to this list MetaMask/metamask-extension#30661

@HowardBraham We discussed that PRs larger than this would require an admin override. Is that sufficient for the edge cases?

cc @sethkfman

[davidmurdoch]

This should probably ignore yarn.lock (and other lock files) and all lavamoat policy file changes, as those can get big really quickly.

[sethkfman]

LGTM

[desi]

This should probably ignore yarn.lock (and other lock files) and all lavamoat policy file changes, as those can get big really quickly.

@davidmurdoch Since this is a shared workflow and other repos might not have lavamoat files should we maybe make the regex for exclude files a param that could be passed in from the calling action?

[davidmurdoch]

@davidmurdoch Since this is a shared workflow and other repos might not have lavamoat files should we maybe make the regex for exclude files a param that could be passed in from the calling action?

Yeah, that sounds like the best way to do this.

[desi]

@davidmurdoch Since this is a shared workflow and other repos might not have lavamoat files should we maybe make the regex for exclude files a param that could be passed in from the calling action?

Yeah, that sounds like the best way to do this.

Added in new ignore_pattern to the inputs and set .lock files as the default. Calling actions can set whatever other files they would like.

[seaona]

mmm so we are fetching the entire repo here right? In Extension, we avoided doing a full fetch, and instead we perform a fetch with depth, to optimize the flow here and we also have this function which increments the depth, in case it's needed (for doing the git-diff operation later on)

I'm wondering if we could re-use some of this? Otherwise we are performing a git fetch with depth, but with this flow, were are now also performing a full fetch 🤔

[desi]

So would the change here be to update row 46 to git fetch --depth 1 origin "$BASE_BRANCH"? I don't think I need the function referenced but please correct me if I am missing something.

[Gudahtt]

I think you would need to fetch a depth of more than 1, as you need all commits in-between the base branch and the HEAD to be present. I cannot recall the behavior of git diff when the commits needed for the diff are missing, but surely it will either error or return suboptimal results without the commits present that are needed for the diff. I think it errors.

In the function @seaona linked, we increment the depth until the command git merge-base origin/HEAD HEAD succeeds, as the goal is to at least have commits up to the merge-base present. I believe a similar strategy should work here as well. Except that instead of origin/HEAD we may want to use origin/$BASE_BRANCH.

[Gudahtt]

I will say though that fetching the whole repository doesn't take that long. Personally I'd consider this a non-blocking suggestion/optimization, which we could do later.

[desi]

@Gudahtt and @seaona I believe I addressed the feedback.