Created on 2021-07-08.14:27:03 by jendrik, last changed by jendrik.
msg10659 (view) |
Author: jendrik |
Date: 2022-03-10.10:50:17 |
|
Alright, merged :-)
|
msg10658 (view) |
Author: malte |
Date: 2022-03-10.10:43:54 |
|
From my side too.
|
msg10657 (view) |
Author: silvan |
Date: 2022-03-10.10:20:58 |
|
From my side, yes.
|
msg10656 (view) |
Author: jendrik |
Date: 2022-03-09.21:36:39 |
|
Yes, I think this change will be beneficial for everyone who pushes Fast Downward clones to GitHub, since they will not exhaust their Actions quota. And it will save lots of unneeded CPU cycles :-)
I merged with the latest revision and added a changelog entry.
Can I go ahead and merge this?
|
msg10638 (view) |
Author: thomas |
Date: 2022-03-07.15:51:24 |
|
I'm OK if this is merged as initially proposed. It won't do anything for me (neither good nor bad), so as long as it improves things for someone else I'm happy with this change.
|
msg10637 (view) |
Author: malte |
Date: 2022-03-07.14:23:10 |
|
From my side, it's enough if someone is happy and nobody is unhappy with it, but from the discussion it looks like maybe the discussion between Jendrik and Thomas is not yet resolved. If we want to integrate this, it needs a changelog entry.
|
msg10636 (view) |
Author: silvan |
Date: 2022-03-07.12:43:28 |
|
Thomas also seemed to like it, didn't he? I agree that it would be good to not always run the full set of actions on each commit, but I can't think of any simple criterion to exclude certain commits from being tested.
|
msg10635 (view) |
Author: jendrik |
Date: 2022-03-07.12:13:49 |
|
I still like the proposed change in the pull request, but so far it seems to be only me :-)
|
msg10634 (view) |
Author: malte |
Date: 2022-03-07.11:38:10 |
|
Hi all, is this stuck in discussion? Any way to get it unstuck? Or have you given up on this?
|
msg10364 (view) |
Author: jendrik |
Date: 2021-07-12.17:39:54 |
|
I also don't know a way of achieving this. Note also, that not all issues require tags at all. In these cases, any push could be the one we want to integrate, so we need to test all pushes, regardless of whether tests for earlier pushes failed or not.
|
msg10363 (view) |
Author: thomas |
Date: 2021-07-12.17:18:28 |
|
I agree that what I proposed won't work well in practice.
My issue with the initial proposal is that it simply won't do anything for me because I usually either work on issues or work on a Fast Downward clone where I disable github actions right after cloning (I won't have to do that anymore, so it will safe me a little bit of effort).
If it is somehow possible, I would like it if github actions only run on a subset of the commits to an issue branch, and for me personally the subset of tagged commits would be perfect. However, I agree that tagging a commit just to see if github actions work is not a good solution at all. With this in mind, a solution that runs github actions
1. if the commit is tagged or
2. on any commit if the last github action failed
would be nice, but I'm not very confident this can be achieved.
|
msg10362 (view) |
Author: jendrik |
Date: 2021-07-09.11:40:32 |
|
I'm having a hard time figuring out the meaning of the docs, so I'm not sure that your configuration from msg10361 would behave as you'd like it to. But let's discuss what we want first before figuring out how to achieve that.
Instead of all pushes to an issue branch, you're proposing to test only the pushes whose last commit is tagged with "issue*", right? In this setup, let's assume that issue2000-v2 fails the tests. Wouldn't we then have to create tag issue2000-v3 just to check that the tests pass now? That sounds a bit tedious to me. Also, note that issue2000-v3 will only be tested if there's no later commit in the push.
|
msg10361 (view) |
Author: thomas |
Date: 2021-07-08.18:37:02 |
|
Yes, you're right, branch requires the push or pull_request trigger. What about using the "tags" configuration:
on:
push:
branches: [main, release-*]
pull_request:
branches: [main, release-*]
tags: [issue*]
(This requires tags to also follow a naming pattern, which I am not sure is the case for everyone).
Additionally, we could use "paths-ignore" to exclude tests for commits that only affect parts of the repository that aren't tested anyway (not sure if this exists).
That said, I'm also fine with going with the solution as proposed initially by Jendrik if it is useful for some, it just will rarely do anything for me.
|
msg10360 (view) |
Author: jendrik |
Date: 2021-07-08.18:14:38 |
|
Yes, my explanation was off. But still, I think "branches" can only be set for "push" and "pull_request": https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestbranchestags
|
msg10358 (view) |
Author: thomas |
Date: 2021-07-08.18:00:39 |
|
I cannot find what you are referring to. The third example on that page uses "page_build" and "release", and clicking on "Events that trigger workflows" in the menu on the left shows me many options. Am I missing something?
|
msg10357 (view) |
Author: jendrik |
Date: 2021-07-08.17:51:54 |
|
I don't think the syntax allows for this (https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions), since "on" can only be followed by "push" and/or "pull_request".
|
msg10356 (view) |
Author: thomas |
Date: 2021-07-08.15:40:30 |
|
I agree this is about personal preferences. Personally, I even find my previous suggestion to busy and, now that I looked into some of the possibilities of triggering github actions, I'd even go with the following:
on:
push:
branches: [main, release-*]
pull_request:
branches: [main, release-*]
create:
branches: [main, issue*, release-*]
If I got this right, "create" triggers the actions when either a branch is created or when a tag is created, which (at least for me) is a relevant subset of commits that I'd like to be checked (and which would significantly decrease the build minutes I'm spending).
|
msg10355 (view) |
Author: jendrik |
Date: 2021-07-08.14:50:20 |
|
I think this is a matter of personal preference. I'm indifferent and can live with both solutions.
|
msg10353 (view) |
Author: thomas |
Date: 2021-07-08.14:42:00 |
|
Is it "issue*" on push really necessary? As long as I haven't opened a pull request, I'd rather not have these checks run all the time, and since we always create a pull request before merging there will always be a test before the actual merging.
|
msg10352 (view) |
Author: jendrik |
Date: 2021-07-08.14:39:37 |
|
In today's meeting we agreed that the change makes sense for us.
Can someone please inspect the pull request at https://github.com/aibasel/downward/pull/56 ?
|
msg10351 (view) |
Author: jendrik |
Date: 2021-07-08.14:27:02 |
|
Currently, all Fast Downward GitHub clones run all CI tests. This quickly consumes the build minutes quota for private repositories and is probably overkill for most public repositories. To save people the trouble from exhausting their build minutes, having to disable the GitHub Actions manually and most importantly to avoid unnecessary compute, I propose to run the tests only for specific branches, i.e., I propose to change
on: [push, pull_request]
to
on:
push:
branches: [main, issue*, release-*]
pull_request:
branches: [main, issue*, release-*]
|
|
Date |
User |
Action |
Args |
2022-03-10 10:50:17 | jendrik | set | status: reviewing -> resolved messages:
+ msg10659 |
2022-03-10 10:43:54 | malte | set | messages:
+ msg10658 |
2022-03-10 10:20:58 | silvan | set | messages:
+ msg10657 |
2022-03-09 21:36:39 | jendrik | set | messages:
+ msg10656 |
2022-03-07 15:51:24 | thomas | set | messages:
+ msg10638 |
2022-03-07 14:23:10 | malte | set | messages:
+ msg10637 |
2022-03-07 12:43:28 | silvan | set | messages:
+ msg10636 |
2022-03-07 12:13:49 | jendrik | set | messages:
+ msg10635 |
2022-03-07 11:38:10 | malte | set | messages:
+ msg10634 |
2021-07-12 17:39:57 | jendrik | set | status: chatting -> reviewing messages:
+ msg10364 summary: Pull request: https://github.com/aibasel/downward/pull/56 |
2021-07-12 17:18:29 | thomas | set | messages:
+ msg10363 |
2021-07-09 11:40:33 | jendrik | set | messages:
+ msg10362 |
2021-07-08 18:37:03 | thomas | set | messages:
+ msg10361 |
2021-07-08 18:14:38 | jendrik | set | messages:
+ msg10360 |
2021-07-08 18:00:39 | thomas | set | messages:
+ msg10358 |
2021-07-08 17:51:56 | jendrik | set | messages:
+ msg10357 |
2021-07-08 15:48:24 | silvan | set | nosy:
+ silvan |
2021-07-08 15:40:31 | thomas | set | messages:
+ msg10356 |
2021-07-08 14:50:20 | jendrik | set | status: unread -> chatting messages:
+ msg10355 |
2021-07-08 14:42:00 | thomas | set | status: reviewing -> unread messages:
+ msg10353 |
2021-07-08 14:39:37 | jendrik | set | status: unread -> reviewing messages:
+ msg10352 |
2021-07-08 14:31:45 | thomas | set | nosy:
+ thomas |
2021-07-08 14:27:03 | jendrik | create | |
|