getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
761 stars 239 forks source link

Use path filter for GH actions #1244

Closed philipphofmann closed 10 months ago

philipphofmann commented 1 year ago

Description

When opening https://github.com/getsentry/sentry-dart/pull/1239, which only touches the PR request template, I noticed that GH actions run almost all checks, which is unnecessary.

We can use path filters for some of the workflows like flutter.yml or e2e_dart.yml to avoid unneeded GH action runs.

You can have a look at Cocoa 1, Cocoa 2 for reference

marandaneto commented 1 year ago

We do have a few path filters, but indeed, they can be improved, thanks @philipphofmann

denrase commented 1 year ago

@philipphofmann I have two questions where you hopefully have more experience than me:

First

All of our PRs are set on pull_request. According to GitHub documentation, paths should not be used in those cases, as the PRs would get stuck in a pending state.

Should we still use them? And if so, do we need to change the trigger from pull_request?

Second

Also, we do use multiple paths-ignore. For example, file looks like this:

pull_request:
  paths-ignore:
    - "**/*.md"
    - "logging/**"
    - "flutter/**"
    - "dio/**"
    - "sqflite/**"

The behaviour per GitHub documentation:

When all the path names match patterns in paths-ignore, the workflow will not run. If any path names do not match patterns in paths-ignore, even if some path names match the patterns, the workflow will run.

So that is the reason why your change to .github/pull_request_template.md triggered all the workflows, as it was a match outside of the provided ignores?

Conclusion

Would you agree that, assuming we can use paths regardless of the first point, we should move to the includes rather than using the ignores?

philipphofmann commented 1 year ago

All of our PRs are set on pull_request. According to GitHub documentation, paths should not be used in those cases, as the PRs would get stuck in a pending state.

Only if you have required checks. On Cocoa, we don't because we want to be able to merge changelog changes quickly, for example.

So that is the reason why your change to .github/pull_request_template.md triggered all the workflows, as it was a match outside of the provided ignores?

I think so, yes.

Without looking into all GH actions properly, I can't tell you which configuration you should use exactly. The goal should be that you only run the jobs that make sense. For example, for a Changelog entry change, GH actions shouldn't run all workflows for a PR. Our changelog changes only run four checks; see https://github.com/getsentry/sentry-cocoa/pull/3334. We achieve this by using path filters for PRs. Please choose whatever makes for this repo. @denrase, does that help you, or do you need further info?

denrase commented 11 months ago

@philipphofmann Thank you for your insights, finally came around to take a look at this. 🙇