23 Sep 2024 |
Pol | Quick question, how do you usually test PR ? Do you cherry pick the commit in your branch or do another trick ? | 07:32:17 |
Genghiz | In reply to @drupol:matrix.org Quick question, how do you usually test PR ? Do you cherry pick the commit in your branch or do another trick ? I use nixpkgs-review | 09:37:35 |
Pol | But in this case, you need to test this against fireflii, how do you do then? | 09:37:58 |
Genghiz | Nixpkgs-review pr -p firefly-iii
| 09:38:22 |
Pol | nice | 09:39:29 |
Pol | I just notice a small typo | 09:39:35 |
Pol | I will fix it after that PR is merged. | 09:39:42 |
Pol | setComposeRootVersion -> setComposerRootVersion | 09:40:01 |
Pol | setComposeEnvVariables -> setComposerEnvVariables | 09:40:12 |
Pol | Let me do it within this PR | 09:40:23 |
Genghiz | So firefly-iii seems to be fixed. | 09:41:06 |
Pol | Done, I just fixed the typo | 09:41:28 |
Genghiz | Once you merge I’ll close the firefly-iii fix PR. | 09:41:29 |
Pol | Can you try it again? | 09:41:40 |
Pol | And post some feedback in the PR ? | 09:41:45 |
Genghiz | Same PR number? | 09:41:48 |
Pol | yeap | 09:41:54 |
Genghiz | 2 mins | 09:42:10 |
Genghiz | Fwiw the tests work, but I’ll add a properly formatted review message in a bit so just merge post that I think. | 10:21:19 |
Genghiz | This is a backwards compatible change so I would recommend a backport as well. | 10:21:35 |
Genghiz | In reply to @genghiz:cdw.go7box.xyz Fwiw the tests work, but I’ll add a properly formatted review message in a bit so just merge post that I think. Done. | 10:35:26 |
Pol | Merged ~! | 10:59:16 |
Pol | * Merged ! | 10:59:17 |
@patka:envs.net | What about the comment I made? | 11:02:52 |
Pol | Which comment? Did I missed something? | 11:03:55 |
@patka:envs.net | I made a comment on the PR, but it still says pending without a submit button so dont know what went wrong. I'm on mobile in the train now so just
But v1 had COMPOSER_DISABLE_NETWORK = "1"; and I think you forgot to add that in your PR? Or was it purposefully left out? | 11:10:51 |
Pol | Left out on purpose ;) | 11:11:43 |
Pol | No worries | 11:11:44 |
Genghiz | In reply to @genghiz:cdw.go7box.xyz https://github.com/NixOS/nixpkgs/pull/341746 @drupol:matrix.orgHave changed this to not set the variable. It’s a version bump rn. Do have a look.
| 11:47:51 |
Pol | So we can now safely say that buildComposerProject2 is stable :) | 12:53:13 |