-
Notifications
You must be signed in to change notification settings - Fork 385
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
ci: run most tests all the time, again #3390
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,6 @@ on: | |
branches: | ||
- master | ||
pull_request: | ||
paths: | ||
- gnovm/**/*.gno | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would we not run them for just these changes? What does a change in any other part of the codebase have to do with the examples? I'd like to see real examples of this issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #2491 was green in the PR, but was red after being merged, because a txtar test was no longer correct. |
||
- examples/**/*.gno | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
|
@@ -80,7 +77,7 @@ jobs: | |
strategy: | ||
fail-fast: false | ||
matrix: | ||
go-version: [ "1.22.x" ] | ||
go-version: ["1.22.x"] | ||
# unittests: TODO: matrix with contracts | ||
runs-on: ubuntu-latest | ||
timeout-minutes: 10 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,6 @@ on: | |
branches: | ||
- master | ||
pull_request: | ||
paths: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say find the dependency, and add it as a path requirement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say just about the entire repository; maybe an exception for |
||
- gnovm/** | ||
- tm2/** # GnoVM has a dependency on TM2 types | ||
workflow_dispatch: | ||
|
||
jobs: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,6 @@ on: | |
branches: | ||
- master | ||
pull_request: | ||
paths: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are working in a monorepository because we have many different components that depend on each other and a codebase that will likely still be in turmoil for the upcoming years. Even remaining on the "simplest" level of breakages, API changes, components in misc/ and contribs/ like autocounterd, loop, gnodev, and gnogenesis depend on the APIs exposed by the many packages in gno.land, gnovm and tm2. If any of these change, and the build breaks on any of these components, the CI should be red, and the issue fixed immediately in the same PR as the API change. But aside from API breakages, components can change in all kinds of subtle ways that their usages elsewhere might not expect. That's why we have tests to ensure that things keep working and behaving as we expect them, and why if tm2 changes, the entire repository needs to be tested, and I don't see many ways out of that. |
||
- misc/** | ||
workflow_dispatch: | ||
|
||
jobs: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we merge a single md change, we need to run the benchmarks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can make this
**/*.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, this benchmark runs in like < 1 minute now
Why do you care about the number of CI checks being executed, especially on master? Personally, I only care about the time it takes for the whole CI to run. (It's not rhetorical, it's a genuine question to understand if I'm missing something.)