build2-packaging / imgui

Build2 package for imgui
Other
2 stars 5 forks source link

Add docking alternative packages #19

Closed wroyca closed 3 months ago

wroyca commented 4 months ago

This follow the approach discussed previously in https://github.com/build2-packaging/imgui/pull/15

Another idea is to add another submodule (e.g., upstream-docking) that points to the docking branch and create the *-docking packages next to the existing ones. This could work seeing that the two branches are released essentially in lockstep and so for each release we will produce the complete set of packages. Also, this will potentially allow us not to duplicate certain packages (e.g., examples). We haven't done anything like this before, but we could try.

image

Klaim commented 4 months ago

Can this be done separately from the bump of version? Maybe make a separate PR for that?

oh wait you did, nevermind 👍🏽

Rookfighter commented 3 months ago

Looks all good to me, just some minor comments. As a final check before we merge (and after you have closed or fixed my comments) could you post a bdep ci link of the latest commit then? @boris-kolpackov are you okay with maintaining the docking and non-docking packages in the same repo? AFAIK upstream has a docking branch and merges the master branch every time it gets tagged into docking, so the branches should stay in sync. We could release docking and non-docking always together then (and have a single common tag in the package repo).

wroyca commented 3 months ago

Looks all good to me, just some minor comments. As a final check before we merge (and after you have closed or fixed my comments) could you post a bdep ci link of the latest commit then? @boris-kolpackov are you okay with maintaining the docking and non-docking packages in the same repo? AFAIK upstream has a docking branch and merges the master branch every time it gets tagged into docking, so the branches should stay in sync. We could release docking and non-docking always together then (and have a single common tag in the package repo).

Thank for the review! CI:

https://ci.stage.build2.org/@39b39ccc-4e24-47e5-9174-f723dea401df

wroyca commented 3 months ago

For Windows Github CI failing, see https://github.com/actions/runner-images/issues/10004#issuecomment-2159234628

boris-kolpackov commented 3 months ago

are you okay with maintaining the docking and non-docking packages in the same repo?

Yes, I think this is the most sensible approach that matches upstream (i.e., they don't seem to have any plans to merge the two branches).

Sorry for the delay in replying to this.

wroyca commented 3 months ago

@Rookfighter Windows CI is now fixed on GitHub side, so now we have all tests passing; this is ready to go! :beers: :tada:

Klaim commented 3 months ago

I'll proceed and merge as @Rookfighter already approved, thanks @wroyca !