Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docking alternative packages #19

Merged
merged 4 commits into from
Jul 2, 2024
Merged

Add docking alternative packages #19

merged 4 commits into from
Jul 2, 2024

Conversation

wroyca
Copy link
Contributor

@wroyca wroyca commented May 13, 2024

This follow the approach discussed previously in #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
Copy link
Contributor

Klaim commented May 13, 2024

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

oh wait you did, nevermind 👍🏽

@Klaim Klaim requested a review from Rookfighter May 22, 2024 20:24
libimgui-docking/LICENSE.txt Outdated Show resolved Hide resolved
libimgui-docking/README.md Outdated Show resolved Hide resolved
libimgui-docking/imgui/imconfig.h Outdated Show resolved Hide resolved
libimgui-docking/manifest Outdated Show resolved Hide resolved
libimgui-examples-docking/buildfile Outdated Show resolved Hide resolved
libimgui-platform-osx/imgui/buildfile Outdated Show resolved Hide resolved
libimgui-render-metal/imgui/buildfile Outdated Show resolved Hide resolved
@Rookfighter
Copy link
Contributor

Rookfighter commented Jun 11, 2024

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
Copy link
Contributor Author

wroyca commented Jun 11, 2024

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
Copy link
Contributor Author

wroyca commented Jun 11, 2024

For Windows Github CI failing, see actions/runner-images#10004 (comment)

@wroyca wroyca requested a review from Rookfighter June 12, 2024 14:01
@boris-kolpackov
Copy link

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
Copy link
Contributor Author

wroyca commented Jul 2, 2024

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

@Klaim
Copy link
Contributor

Klaim commented Jul 2, 2024

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

@Klaim Klaim merged commit f11e2e5 into build2-packaging:main Jul 2, 2024
10 checks passed
@Klaim Klaim mentioned this pull request Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants