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.

Sign up for GitHub

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

Jump to bottom

add health check job #2266

Merged
merged 4 commits into from
Aug 14, 2024
Merged

add health check job #2266

merged 4 commits into from
Aug 14, 2024

Conversation

jakemac53
Copy link
Contributor

This should cut down on the number of comments we need to leave regarding changelog updates etc.

@mosuem
Copy link
Member

mosuem commented Aug 13, 2024

To work on PRs from forks this also needs another workflow file (comment_on_pr.yaml or similar)

name: Comment on the pull request

on:
  # Trigger this workflow after the Health workflow completes. This workflow will have permissions to
  # do things like create comments on the PR, even if the original workflow couldn't.
  workflow_run:
    workflows: 
      - Health
      - Publish
    types:
      - completed

jobs:
  upload:
    uses: dart-lang/ecosystem/.github/workflows/post_summaries.yaml@main
    permissions:
      pull-requests: write

@jakemac53
Copy link
Contributor Author

To work on PRs from forks this also needs another workflow file (comment_on_pr.yaml or similar)

We should add this to the docs as well? I am a bit confused how exactly this solves the issue though

@mosuem
Copy link
Member

mosuem commented Aug 14, 2024

We should add this to the docs as well? I am a bit confused how exactly this solves the issue though

Thanks for noticing, the docs do need an update.

The issue is that Github does not allow a workflow to comment on a PR directly, if the PR is from a fork. This is for security reasons. We circumvent this by uploading the comment as an artifact, and running a second workflow which downloads the artifact and posts it as a comment.

@mosuem mosuem mentioned this pull request Aug 14, 2024
Merged
1 task
@jakemac53
Copy link
Contributor Author

We circumvent this by uploading the comment as an artifact, and running a second workflow which downloads the artifact and posts it as a comment

What I am confused about is how that second workflow is able to post the comment? Why is it allowed to when the first one is not?

mosuem added a commit to dart-lang/ecosystem that referenced this pull request Aug 14, 2024
@mosuem
Update PR Health install instructions (#286)
de7883c 
As per the comments in dart-lang/test#2266.

Thanks @jakemac53 !

---

- [x] I’ve reviewed the contributor guide and applied the relevant
portions to this PR.

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor
guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md)
for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before
creating a PR.
- Contributions to our repos should follow the [Dart style
guide](https://dart.dev/guides/language/effective-dart) and use `dart
format`.
- Most changes should add an entry to the changelog and may need to [rev
the pubspec package
version](https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change).
- Changes to packages require [corresponding
tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs -
please allow for some latency before initial review feedback.
</details>
@jakemac53
Copy link
Contributor Author

Ah, reading the linked doc in the PR you sent to update the usage explains how this works around the issue 👍

@jakemac53 jakemac53 merged commit cfc18ee into master Aug 14, 2024
51 checks passed
@jakemac53 jakemac53 deleted the health-checks branch August 14, 2024 18:29
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 19, 2024
@devoncarew
[deps] rev dartdoc, ecosystem, http, test, tools
1bed4a8 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/ce09815..b444974):
  b4449742  2024-08-14  Sam Rawlins  Add a doc-import into template_data.dart to resolve 'GeneratorBackend.write' (dart-lang/dartdoc#3832)
  7fb1f3f3  2024-08-14  Sam Rawlins  Fix links in sidebar of extension types (dart-lang/dartdoc#3831)
  392e2aa3  2024-08-14  Sam Rawlins  Simplify some Container fields only used for sidebar logic. (dart-lang/dartdoc#3834)
  31e110d6  2024-08-13  Sam Rawlins  Add tests supporting the wildcard feature (dart-lang/dartdoc#3835)

ecosystem (https://github.com/dart-lang/ecosystem/compare/2719d0c..8626bff):
  8626bff  2024-08-16  Jacob MacDonald  More concise formatting of github workflow summary comments (dart-lang/ecosystem#288)
  de7883c  2024-08-14  Moritz  Update PR Health install instructions (dart-lang/ecosystem#286)
  03bf029  2024-08-14  Moritz  Fix linting (dart-lang/ecosystem#287)

http (https://github.com/dart-lang/http/compare/76512c4..b97b8dc):
  b97b8dc  2024-08-16  Anikate De  pkgs/ok_http: OkHttpClientConfiguration and configurable timeouts. (dart-lang/http#1289)
  4322382  2024-08-13  Brian Quinlan  Fix "unintended_html_in_doc_comment" analysis errors (dart-lang/http#1291)

test (https://github.com/dart-lang/test/compare/8be3c94..cd3dbd5):
  cd3dbd51  2024-08-15  Jacob MacDonald  require approval from core-package-admins for anything under /pkgs (dart-lang/test#2268)
  cfc18ee1  2024-08-14  Jacob MacDonald  add health check job (dart-lang/test#2266)
  f3984a72  2024-08-13  Jacob MacDonald  update changelogs per dart-lang/test#2262 (dart-lang/test#2267)

tools (https://github.com/dart-lang/tools/compare/d563c38..5b15f8b):
  5b15f8b  2024-08-14  Nate Bosch  Add examples of calling two algorithms to README (dart-lang/tools#293)
  ece541c  2024-08-14  Nate Bosch  Ignore unintended_html_in_doc_comment (dart-lang/tools#294)

Change-Id: I23d2f9606e4bfefdfa61e0a4c629f3f05f00c996
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381320
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
@natebosch
Copy link
Member

@jakemac53 - it looks like the "coverage" check re-runs a bunch of tests that we run in other actions. We might want to skip that one always for the test repo since it's so slow.

Do we have a good way to configure that?

@natebosch
Copy link
Member

Do we have a good way to configure that?

Found it. #2270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mosuem mosuem mosuem left review comments

@natebosch natebosch natebosch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jakemac53 @mosuem @natebosch