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

Early exit when no files are changed. #456

Merged
merged 7 commits into from Jul 7, 2023
Merged

Early exit when no files are changed. #456

merged 7 commits into from Jul 7, 2023

Conversation

nathanhammond
Copy link
Contributor

As a consequence of the checkAll function call returning true if the length of changedFiles is 0, this must early-exit in order to avoid labeling empty PRs.

Example labeling failure here: vercel/turbo#2706

@nathanhammond nathanhammond requested a review from a team as a code owner November 15, 2022 07:35
@nathanhammond
Copy link
Contributor Author

The source of the error is here:

labeler/src/labeler.ts

Lines 202 to 214 in 5c75392

function checkAll(changedFiles: string[], globs: string[]): boolean {
const matchers = globs.map((g) => new Minimatch(g));
core.debug(` checking "all" patterns`);
for (const changedFile of changedFiles) {
if (!isMatch(changedFile, matchers)) {
core.debug(` "all" patterns did not match against ${changedFile}`);
return false;
}
}
core.debug(` "all" patterns matched all files`);
return true;
}

The loop on lines 205-210 does not execute and therefore the checkAll check returns true.

@nathanhammond
Copy link
Contributor Author

Rebased and updated to match new code style. As all changes were formatting, the dist asset didn't change.

src/labeler.ts Outdated Show resolved Hide resolved
src/labeler.ts Outdated Show resolved Hide resolved
As a consequence of the `checkAll` function call returning `true` if the length of `changedFiles` is 0, this must early-exit in order to avoid labeling empty PRs.
@nathanhammond nathanhammond requested a review from a team as a code owner February 14, 2023 01:10
@nathanhammond
Copy link
Contributor Author

Updated and rebuilt.

@AndreiLobanovich
Copy link
Contributor

Hello @nathanhammond! Could you please tell me, how do you create an empty PR? The link you provided as a labeler failure leads to a pull request. If you could provide the link with the run in which labeler failed - that would help me investigate the issue.

@nathanhammond
Copy link
Contributor Author

@AndreiLobanovich I didn't create the PR that failed and it has changed a lot since then. I did see the consequences of the failure. You can review the tests that I added to demonstrate the failure in the event there are no changed files. Two commits with a net result of zero changes would probably do it?

(I wouldn't have filed this PR without having seen it fail once. 😜 I wouldn't have known to go look!)

Copy link
Contributor

@AndreiLobanovich AndreiLobanovich left a comment

Choose a reason for hiding this comment

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

Hello, @nathanhammond!
Thanks for your contribution and attention to details! I managed to reproduce the incorrect behavior. Could you please make a few minor changes?

src/labeler.ts Show resolved Hide resolved
src/labeler.ts Outdated Show resolved Hide resolved
src/labeler.ts Outdated Show resolved Hide resolved
@AndreiLobanovich
Copy link
Contributor

Hi @nathanhammond ,

I hope this message finds you well. I wanted to check in and see how you're progressing with this PR.

Please feel free to let me know if you need any assistance or if there's anything that you're unsure of. We greatly appreciate your contribution and are here to support you in any way we can.

Thank you for your time and effort.

@AndreiLobanovich
Copy link
Contributor

Hello, @nathanhammond!

Didn't hear from you for a while. Do you have some spare time to finish work on this PR? Is there anything I can help you with?

@AndreiLobanovich
Copy link
Contributor

Hey, @nathanhammond, we miss you a lot. If you could find some spare time to finish work on this PR that would be great.
Hope you are doing well.

@MaksimZhukov MaksimZhukov merged commit be13bbd into actions:main Jul 7, 2023
7 checks passed
@MaksimZhukov
Copy link
Contributor

@nathanhammond thank you for the contribution!

github-merge-queue bot pushed a commit to owntracks/android that referenced this pull request Jul 11, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/labeler](https://togithub.com/actions/labeler) | action |
minor | `v4.2.0` -> `v4.3.0` |

---

### ⚠ Dependency Lookup Warnings ⚠

Warnings were logged while processing this repo. Please check the logs
for more information.

---

### Release Notes

<details>
<summary>actions/labeler (actions/labeler)</summary>

### [`v4.3.0`](https://togithub.com/actions/labeler/releases/tag/v4.3.0)

[Compare
Source](https://togithub.com/actions/labeler/compare/v4.2.0...v4.3.0)

#### What's Changed

In scope of this release, the ability to specify pull request number(s)
was added by [@&#8203;credfeto](https://togithub.com/credfeto) in
[actions/labeler#349.

Support for reading from the configuration file presented on the runner
was added by [@&#8203;lrstanley](https://togithub.com/lrstanley) in
[actions/labeler#394.
It allows you to use a configuration file generated during workflow run
or uploaded from a separate repository.

Please refer to the [action
documentation](https://togithub.com/actions/labeler#inputs) for more
information.

This release also includes the following changes:

- Improved Error message for missing config file by
[@&#8203;Gornoka](https://togithub.com/Gornoka) in
[actions/labeler#475
- Early exit when no files are changed by
[@&#8203;nathanhammond](https://togithub.com/nathanhammond) in
[actions/labeler#456
- Add examples to match all repo files by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#600
- Fix a typo in the example about using the action outputs by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#606
- Bump eslint from 8.43.0 to 8.44.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#601
- Bump
[@&#8203;typescript-eslint/parser](https://togithub.com/typescript-eslint/parser)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#602
- Bump
[@&#8203;typescript-eslint/eslint-plugin](https://togithub.com/typescript-eslint/eslint-plugin)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#604
- Bump tough-cookie from 4.1.2 to 4.1.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#609
- Bump
[@&#8203;octokit/plugin-retry](https://togithub.com/octokit/plugin-retry)
from 5.0.4 to 5.0.5 by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#610

#### New Contributors

- [@&#8203;credfeto](https://togithub.com/credfeto) made their first
contribution in
[actions/labeler#349
- [@&#8203;lrstanley](https://togithub.com/lrstanley) made their first
contribution in
[actions/labeler#394
- [@&#8203;nathanhammond](https://togithub.com/nathanhammond) made their
first contribution in
[actions/labeler#456

**Full Changelog**:
actions/labeler@v4...v4.3.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/owntracks/android).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi41LjMiLCJ1cGRhdGVkSW5WZXIiOiIzNi41LjMiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
github-merge-queue bot pushed a commit to owntracks/android that referenced this pull request Jul 11, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/labeler](https://togithub.com/actions/labeler) | action |
minor | `v4.2.0` -> `v4.3.0` |

---

### ⚠ Dependency Lookup Warnings ⚠

Warnings were logged while processing this repo. Please check the logs
for more information.

---

### Release Notes

<details>
<summary>actions/labeler (actions/labeler)</summary>

### [`v4.3.0`](https://togithub.com/actions/labeler/releases/tag/v4.3.0)

[Compare
Source](https://togithub.com/actions/labeler/compare/v4.2.0...v4.3.0)

#### What's Changed

In scope of this release, the ability to specify pull request number(s)
was added by [@&#8203;credfeto](https://togithub.com/credfeto) in
[actions/labeler#349.

Support for reading from the configuration file presented on the runner
was added by [@&#8203;lrstanley](https://togithub.com/lrstanley) in
[actions/labeler#394.
It allows you to use a configuration file generated during workflow run
or uploaded from a separate repository.

Please refer to the [action
documentation](https://togithub.com/actions/labeler#inputs) for more
information.

This release also includes the following changes:

- Improved Error message for missing config file by
[@&#8203;Gornoka](https://togithub.com/Gornoka) in
[actions/labeler#475
- Early exit when no files are changed by
[@&#8203;nathanhammond](https://togithub.com/nathanhammond) in
[actions/labeler#456
- Add examples to match all repo files by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#600
- Fix a typo in the example about using the action outputs by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#606
- Bump eslint from 8.43.0 to 8.44.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#601
- Bump
[@&#8203;typescript-eslint/parser](https://togithub.com/typescript-eslint/parser)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#602
- Bump
[@&#8203;typescript-eslint/eslint-plugin](https://togithub.com/typescript-eslint/eslint-plugin)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#604
- Bump tough-cookie from 4.1.2 to 4.1.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#609
- Bump
[@&#8203;octokit/plugin-retry](https://togithub.com/octokit/plugin-retry)
from 5.0.4 to 5.0.5 by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#610

#### New Contributors

- [@&#8203;credfeto](https://togithub.com/credfeto) made their first
contribution in
[actions/labeler#349
- [@&#8203;lrstanley](https://togithub.com/lrstanley) made their first
contribution in
[actions/labeler#394
- [@&#8203;nathanhammond](https://togithub.com/nathanhammond) made their
first contribution in
[actions/labeler#456

**Full Changelog**:
actions/labeler@v4...v4.3.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/owntracks/android).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi41LjMiLCJ1cGRhdGVkSW5WZXIiOiIzNi41LjMiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
flookapa pushed a commit to flookapa/owntracks_android that referenced this pull request Aug 27, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [actions/labeler](https://togithub.com/actions/labeler) | action |
minor | `v4.2.0` -> `v4.3.0` |

---

### ⚠ Dependency Lookup Warnings ⚠

Warnings were logged while processing this repo. Please check the logs
for more information.

---

### Release Notes

<details>
<summary>actions/labeler (actions/labeler)</summary>

### [`v4.3.0`](https://togithub.com/actions/labeler/releases/tag/v4.3.0)

[Compare
Source](https://togithub.com/actions/labeler/compare/v4.2.0...v4.3.0)

#### What's Changed

In scope of this release, the ability to specify pull request number(s)
was added by [@&#8203;credfeto](https://togithub.com/credfeto) in
[actions/labeler#349.

Support for reading from the configuration file presented on the runner
was added by [@&#8203;lrstanley](https://togithub.com/lrstanley) in
[actions/labeler#394.
It allows you to use a configuration file generated during workflow run
or uploaded from a separate repository.

Please refer to the [action
documentation](https://togithub.com/actions/labeler#inputs) for more
information.

This release also includes the following changes:

- Improved Error message for missing config file by
[@&#8203;Gornoka](https://togithub.com/Gornoka) in
[actions/labeler#475
- Early exit when no files are changed by
[@&#8203;nathanhammond](https://togithub.com/nathanhammond) in
[actions/labeler#456
- Add examples to match all repo files by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#600
- Fix a typo in the example about using the action outputs by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#606
- Bump eslint from 8.43.0 to 8.44.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#601
- Bump
[@&#8203;typescript-eslint/parser](https://togithub.com/typescript-eslint/parser)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#602
- Bump
[@&#8203;typescript-eslint/eslint-plugin](https://togithub.com/typescript-eslint/eslint-plugin)
from 5.60.1 to 5.61.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#604
- Bump tough-cookie from 4.1.2 to 4.1.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[actions/labeler#609
- Bump
[@&#8203;octokit/plugin-retry](https://togithub.com/octokit/plugin-retry)
from 5.0.4 to 5.0.5 by
[@&#8203;MaksimZhukov](https://togithub.com/MaksimZhukov) in
[actions/labeler#610

#### New Contributors

- [@&#8203;credfeto](https://togithub.com/credfeto) made their first
contribution in
[actions/labeler#349
- [@&#8203;lrstanley](https://togithub.com/lrstanley) made their first
contribution in
[actions/labeler#394
- [@&#8203;nathanhammond](https://togithub.com/nathanhammond) made their
first contribution in
[actions/labeler#456

**Full Changelog**:
actions/labeler@v4...v4.3.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/owntracks/android).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi41LjMiLCJ1cGRhdGVkSW5WZXIiOiIzNi41LjMiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.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.

None yet

5 participants