24 Sep 2024 |
saranshbaniyal | yes it makes more sense to only have it triggered when there is a push on master, but then again as you mentioned we have to check that edge case during PR | 12:16:54 |
viferga | good job, do the change and I will merge it this night so we can see if it works properly | 12:17:31 |
saranshbaniyal | I have added the changes, but I think if: github.ref == 'refs/heads/master this step would prevent it from executing from the needs section on PR, please do let me know if it fails | 12:26:43 |
viferga | that's the correct behavior | 12:29:21 |
viferga | check the dependency tree here: https://github.com/metacall/install/pull/28#issuecomment-2363555374 | 12:30:29 |
viferga | if you change the first step by PR instead of commit to master, the second step does not work, the code is not pushed in master and the image of the cli won't be built with the latest changes | 12:31:32 |
viferga | we won't be able to test that during PR but I cannot find a better alternative, the PR will be tested after merge | 12:35:56 |
saranshbaniyal | I have one doubt, by the code is not pushed in master and the image of the cli won't be built with the latest changes do you mean this won't happen because in the https://github.com/metacall/cli/blob/c6bdb48c1c55731775150353da44696c64653c3d/.github/workflows/docker-hub.yml it is mentioned to get triggered on push to master of metacall/cli ? | 12:58:28 |
saranshbaniyal | * I have one doubt, by the code is not pushed in master and the image of the cli won't be built with the latest changes do you mean this won't happen because in the https://github.com/metacall/cli/blob/c6bdb48c1c55731775150353da44696c64653c3d/.github/workflows/docker-hub.yml it is mentioned to get push latest image to dockerhub only when there is a push to master of metacall/cli ? | 12:59:25 |
viferga | no, it's because of this line: | 13:01:04 |
viferga | https://github.com/metacall/cli/blob/c6bdb48c1c55731775150353da44696c64653c3d/Dockerfile#L36 | 13:01:43 |
viferga | this always uses the install.sh from master | 13:01:58 |
viferga | if we trigger this during the PR it will download the master one, which is in a different branch.. | 13:02:28 |
saranshbaniyal | oh I get it now, thanks for explaining this 😄 | 13:10:22 |
viferga | we can pass the url as input for the other CI if we want to run it with external PRs | 13:11:15 |
viferga | I mean, other branches/forks | 13:11:35 |
saranshbaniyal | from within the .yml file? | 13:12:48 |
viferga | check out client_payload here: https://github.com/convictional/trigger-workflow-and-wait | 13:15:08 |
viferga | and now read this: https://graphite.dev/guides/github-actions-inputs | 13:17:11 |
saranshbaniyal | understood | 13:38:43 |
25 Sep 2024 |
viferga | saranshbaniyal I ran it and it failed, I am going to review it later on.. I am solving other issues now | 19:24:26 |
viferga | https://github.com/metacall/install/pull/28#issuecomment-2375140689 | 20:01:23 |
28 Sep 2024 |
| @dfhui8:matrix.org joined the room. | 10:08:26 |
| @dfhui8:matrix.org joined the room. | 10:15:29 |
| @dfhui8:matrix.org left the room. | 10:16:05 |
| @ghuik6:matrix.org joined the room. | 11:53:47 |
| @ghuik6:matrix.org joined the room. | 12:01:31 |
| @ghuik6:matrix.org left the room. | 12:02:14 |
30 Sep 2024 |
| prkteja changed their profile picture. | 16:14:45 |
2 Oct 2024 |
| fisher6699 joined the room. | 05:15:32 |