juhp / fbrnch

Tool to update fedora packages branches
https://hackage.haskell.org/package/fbrnch
GNU General Public License v2.0
23 stars 3 forks source link

[Suggestion] create-review and update-review: Disable scratch builds by default #43

Closed farchord closed 1 year ago

farchord commented 1 year ago

This is just a suggestion, but with Fedora's auto build system now going through all the review requests, could we set a default behavior to disable automatic scratch builds on new review submissions and updates?

Or at least allow an option to be set to change the behavior locally.

juhp commented 1 year ago

Good idea thanks

juhp commented 1 year ago

I have been thinking about this a little one and off in the back of my head... Your suggestion certainly makes some sense, on the other hand having the koji scratch build sanity check before uploading still seems attractive to me, though I agree that the review auto building provides more (later). IWBNI fbrnch could trigger that directly instead perhaps - though not sure how yet. I feel earlier feedback is always more useful: another alternative could be to trigger a local mock build instead of koji scratch.

To date I have hesitated to add a config file for fbrnch but it will probably be needed sooner or later.

farchord commented 1 year ago

To be honest, right now I've mostly just always been feeding in the -S parameter. Maybe a question:

"Would you like to perform a scratch build? (Y/N)"

Once you implemented configuration files, maybe it could remember the choice.

farchord commented 1 year ago

I would also argue that doing a mockbuild instead is probably not the best of ideas as, usually, when you create a package you have to do it a couple times anyway to make sure everything works. You invoke fbrnch once that's done to create the review, it's not really necessary.

Proving that your package builds is a good idea though, but with the Fedora-review-bot working now isn't it redundant?

juhp commented 1 year ago

Well the thing is one may have under-specified BRs for example (installed locally).

Anyway I take your point overall and so let's default to no scratch builds then for now, since it seems the simplest/least-surprising after all.

juhp commented 1 year ago

Okay I have code for this too, which I can push soon

(edited) Are automated build failures flagged in bz btw?

farchord commented 1 year ago

Okay I have code for this too, which I can push soon

Do scratch build failures show up in bz with the automated builds btw?

In 99% of cases, I don't do scratch builds so I couldn't tell you. I've mostly used fbrnch for KF6 packages which was pretty cut and dry and easily reviewed. And I always locally mockbuild.

juhp commented 1 year ago

Okay I was asking for the automated builds... ((fbrnch won't create/update a review if its scratch build failed))

juhp commented 1 year ago

Okay I pushed a review request today: https://copr.fedorainfracloud.org/coprs/g/fedora-review/fedora-review-2244987-ghc-text-icu/build/6546489/

So yes it does show whether the copr build succeeded: the worse thing is that it is only built for x86_64 so without the koji build there is no way to know that it will succeed on other archs (until after approved?)?

This makes me think maybe a prompt is more appropriate after all. Well for noarch packages the copr build is sufficient I think.

But I can default the prompt to no as a middle-way.

juhp commented 1 year ago

This is in just released 1.3.3