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

Ignore paths unrelated to builds in CI #6789

Merged
merged 4 commits into from
Aug 2, 2021
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,22 @@ on:
branches:
- dev
- master
paths-ignore:
- 'README*.md'
Copy link
Member

Choose a reason for hiding this comment

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

Would it be a good idea to ignore all *.md files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, but I tried to be conservative here so so that there are no false negatives.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I still don't feel glad about picking so many paths explicitly.

I don't think there can be any false positives (at least currently):

All .md files are either Readme.md's or stuff in the .github folder:
https://github.com/TeamNewPipe/NewPipe/search?l=Markdown&p=1

So maybe just go with the simpler **.md?
Or what about simply ignoring all .md's in the .github folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My concern is that somebody might change something in the code to depend on one of these files (or adds one), and then they forgot to change the paths listed here. However, I'm not strongly opposed to being less conservative and ignoring more things.

Copy link
Member

Choose a reason for hiding this comment

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

Okay.

However consider also the opposite situation could occur.
e.g. someone adds a new .md to the .github folder and thinks it's ignore by the CI.

Anyway, I'm also fine with the current method.
But can you at least also add .github/CONTRIBUTING.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

someone adds a new .md to the .github folder and thinks it's ignore by the CI.

Not everybody is necessarily aware that files that the CI ignore actually exist, so it's more likely for a mistake to happen in this case I'd say. Anyway, I'm going to add .github/**/*.md then.

- 'fastlane/**'
- 'assets/**'
- .github/PULL_REQUEST_TEMPLATE.md
triallax marked this conversation as resolved.
Show resolved Hide resolved
- '.github/ISSUE_TEMPLATE/**'
push:
branches:
- dev
- master
paths-ignore:
- 'README*.md'
- 'fastlane/**'
- 'assets/**'
- .github/PULL_REQUEST_TEMPLATE.md
- '.github/ISSUE_TEMPLATE/**'

jobs:
build-and-test-jvm:
Expand Down