Skip to content

Add logic to skip files with more than 15000 lines#186

Merged
ariskataoka merged 5 commits intomasterfrom
feature/skip-files-15000-lines-rebase
Jul 15, 2021
Merged

Add logic to skip files with more than 15000 lines#186
ariskataoka merged 5 commits intomasterfrom
feature/skip-files-15000-lines-rebase

Conversation

@ariskataoka
Copy link
Member

TLDR; Some builds are currently timing out due to PRs that contain too large files.

This PR addresses issue 147.

If the PR contains files with more than 15000 lines, the lint scan and the phpcs will skip it.

  • Adds logic to validate file size;
  • Adds logic to modify the bot message;
  • Adds logic to ignore files that have been skipped previously;

TODO:

  • [ X ] Implement [new feature / logic]
  • [ X ] Add/update unit-tests
  • [ X ] Manual testing
  • [ X ] Pull-Request with files with more than 15000 lines

Obs.:

  1. Tested manually with lint and php-cs on and off: Only exceeded lines failed - cache tests ariskataoka/vipgoci-tests#10 (review)

  2. The build testing has constantly been failing due to the GH API limit rate.

[ 2021-05-26T08:37:48+00:00 -- 0 ]  Ran out of request limits for GitHub, cannot continue without making further requests.; {
    "github_url": "https:\/\/api.github.com\/repos\/automattic\/vip-go-ci-testing\/pulls\/39\/commits?page=1&per_page=100",
    "x-ratelimit-remaining": "1",
    "x-ratelimit-limit": "60"
}
  1. Original branch/PR here: Skip files when they are more than 15000 lines #176

@ariskataoka ariskataoka requested a review from gudmdharalds as a code owner July 2, 2021 08:05
@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch from e2386ee to b59b1c1 Compare July 5, 2021 07:22
Copy link
Contributor

@gudmdharalds gudmdharalds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good!

I left only a few comments inline.

@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch 2 times, most recently from 0f653a8 to c01757d Compare July 6, 2021 02:21
Copy link
Collaborator

@wpcomvip-vipgoci-bot wpcomvip-vipgoci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpcs scanning turned up:

🚫 1 error


This bot provides automated PHP linting and PHPCS scanning. For more information about the bot and available customizations, see our documentation.

@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch from c1a7c58 to 65cc116 Compare July 8, 2021 02:13
@wpcomvip-vipgoci-bot wpcomvip-vipgoci-bot dismissed their stale review July 8, 2021 02:17

Dismissing review as all inline comments are obsolete by now

@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch 3 times, most recently from 8fb7e33 to 12f4376 Compare July 8, 2021 05:09
Copy link
Contributor

@gudmdharalds gudmdharalds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I reviewed and this PR is looking excellent.

There is just one small issue I noted, and when that is resolved this PR can be merged.

Can you have a look?

Thanks!

@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch 2 times, most recently from 46d6324 to fa34e04 Compare July 12, 2021 09:45
History:

Add validation

Add file validation

Introduce file validation before phpcs scan

Add submit message about skipped files to GH

Update validation

Refactor to apply validation as an array

Remove unnecessary function

Fix unit test

Add require for file validation

Modify test to assert equals instead of same

Currently, it's been trying to assert two different objects, when
the object only need to have the same attribute values

Fix small issue

Stops forcing for tests

Fix array structure for svg scan

Add skip-file logic into a separated file

Add validation/skip files logic in the lint scan

Add verification to check if the file is skipped

Add back code block removed by mistake during rebase

Add test for skipped files due to limit of lines exceeded

Return 250 when skipped files due to files exceeded issue

Add tests for skip files funtions

Move message functions for skipped files to skip-file

Adds test

Address bot comments

Add filename in the output for file validation

Add comment to explain why bot returns non-zero when find skipped files

Add code removed by rebase

Add strict types/return types for the new files.

Sanitizes and verify if line count command output is numeric

Fix test mock

Fix verification if files has been already scanned in php-cs process

Add cache layer to file validation

Replace assertequals with assertsame

Refactor cache logic to use cache key as array and value as string
Default is set to 15000
@ariskataoka ariskataoka force-pushed the feature/skip-files-15000-lines-rebase branch from fa34e04 to 04df222 Compare July 13, 2021 00:03
@ariskataoka ariskataoka added this to the 1.0.8 milestone Jul 13, 2021
ariskataoka added a commit that referenced this pull request Jul 13, 2021
@ariskataoka ariskataoka merged commit 8ab0feb into master Jul 15, 2021
@ariskataoka ariskataoka deleted the feature/skip-files-15000-lines-rebase branch August 20, 2021 08:06
ariskataoka added a commit that referenced this pull request Aug 25, 2021
* Adding changelog entry

* Updating version

* Update merge SHA

* Add PR #186 to the changelog

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Ariana Kataoka <aris.kataoka@gmail.com>
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.

3 participants