BuddiesOfBudgie/budgie-desktop-services
5
0
Fork 1
Code Issues 1 Pull requests Projects Releases 2 Activity Actions

Replace generated dbus source files #19

Closed
fossfreedom wants to merge 4 commits from fossfreedom/budgie-desktop-services:autogendbus into main
pull from: fossfreedom/budgie-desktop-services:autogendbus
merge into: BuddiesOfBudgie:main
Conversation 10 Commits 4 Files changed 21 +91 -995
fossfreedom commented 2025-12-04 23:40:25 +01:00
Owner
Copy link

This PR replaces the in source generated dbus files with files created from the schema XML files.

This PR replaces the in source generated dbus files with files created from the schema XML files.
JoshStrobl requested changes 2025-12-05 00:05:56 +01:00
JoshStrobl left a comment
Owner
Copy link

Thanks for the pull request! Some bits of feedback:

  1. I think we should move the added configuration of dbus generation into a dedicated CMakeLists.txt in src/dbus to keep the one in src tidier.
  2. You have an empty .gitignore in src/dbus/generated. Guessing that was originally added when the idea was maybe to dump it into the SOURCE_DIR but given it's being thrown into the directory generator by cmake, probably okay to not have it?
  3. Let's clean up the Taskfile.yml to remove the qdbus-gen task since we'll be using CMake for this.
Thanks for the pull request! Some bits of feedback: 1. I think we should move the added configuration of dbus generation into a dedicated CMakeLists.txt in `src/dbus` to keep the one in `src` tidier. 2. You have an empty `.gitignore` in src/dbus/generated. Guessing that was originally added when the idea was maybe to dump it into the SOURCE_DIR but given it's being thrown into the directory generator by cmake, probably okay to not have it? 3. Let's clean up the `Taskfile.yml` to remove the qdbus-gen task since we'll be using CMake for this.
ngompa commented 2025-12-05 01:13:59 +01:00
Owner
Copy link

The .gitignore file keeps the folder in git, no files means no folder.

The `.gitignore` file keeps the folder in git, no files means no folder.
fossfreedom commented 2025-12-06 10:46:53 +01:00
Author
Owner
Copy link

I think we should move the added configuration of dbus generation into a dedicated CMakeLists.txt in src/dbus to keep the one in src tidier.

Tried different ways - there doesnt seem to be a way to get the order of the build correct with a separate src/dbus/CMakeLists.txt - the autogen is the problem. It is run after the compilation starts and I can't find a way to force the autogen files to be done upfront

You have an empty .gitignore in src/dbus/generated. Guessing that was originally added when the idea was maybe to dump it into the SOURCE_DIR but given it's being thrown into the directory generator by cmake, probably okay to not have it?
Let's clean up the Taskfile.yml to remove the qdbus-gen task since we'll be using CMake for this.

Done

> I think we should move the added configuration of dbus generation into a dedicated CMakeLists.txt in src/dbus to keep the one in src tidier. Tried different ways - there doesnt seem to be a way to get the order of the build correct with a separate src/dbus/CMakeLists.txt - the autogen is the problem. It is run after the compilation starts and I can't find a way to force the autogen files to be done upfront >You have an empty .gitignore in src/dbus/generated. Guessing that was originally added when the idea was maybe to dump it into the SOURCE_DIR but given it's being thrown into the directory generator by cmake, probably okay to not have it? Let's clean up the Taskfile.yml to remove the qdbus-gen task since we'll be using CMake for this. Done
ngompa commented 2025-12-06 10:51:29 +01:00
Owner
Copy link

We've now lost the dbus/generated directory since there's no files in it. Git only keeps directories with files.

We've now lost the `dbus/generated` directory since there's no files in it. Git only keeps directories with files.
fossfreedom commented 2025-12-06 10:59:54 +01:00
Author
Owner
Copy link

the generated files are now in the build/src/dbus/generated folder - i.e. only needed for the compilation.

the generated files are now in the build/src/dbus/generated folder - i.e. only needed for the compilation.
EbonJaeger reviewed 2025-12-06 16:54:53 +01:00
src/CMakeLists.txt Outdated
@ -12,6 +12,92 @@ ecm_add_qtwayland_client_protocol(
protocols/wlr-output-management-unstable-v1.xml BASENAME
wlr-output-management-unstable-v1)
# ============================================================================
EbonJaeger commented 2025-12-06 16:54:53 +01:00
Owner
Copy link

This all doesn't feel right to me... It seems way too complicated. Maybe see what plasma-workspace does for libnotificationmanager: https://invent.kde.org/plasma/plasma-workspace/-/blob/master/libnotificationmanager/CMakeLists.txt?ref_type=heads#L55

This all doesn't feel right to me... It seems way too complicated. Maybe see what `plasma-workspace` does for `libnotificationmanager`: https://invent.kde.org/plasma/plasma-workspace/-/blob/master/libnotificationmanager/CMakeLists.txt?ref_type=heads#L55
fossfreedom marked this conversation as resolved 
Signed-off-by: Evan Maddock <maddock.evan@vivaldi.net>
fossfreedom commented 2025-12-06 21:59:12 +01:00
Author
Owner
Copy link

Builds nicely Evan - cheers!

Builds nicely Evan - cheers!
EbonJaeger commented 2025-12-07 02:05:06 +01:00
Owner
Copy link

Quick question: Was it intentional to completely remove the Taskfile? I thought the idea was just to remove the one task, not all of them.

Quick question: Was it intentional to completely remove the Taskfile? I thought the idea was just to remove the one task, not all of them.
JoshStrobl commented 2025-12-09 17:41:37 +01:00
Owner
Copy link

Yea only the specific task should've been deleted, not the entire thing.

Yea only the specific task should've been deleted, not the entire thing.
JoshStrobl commented 2025-12-09 17:42:13 +01:00
Owner
Copy link

Also this is gonna be hell to rebase on top of #18, might be easier to have a branch from scratch once it's landed 😁

Also this is gonna be hell to rebase on top of #18, might be easier to have a branch from scratch once it's landed 😁
fossfreedom closed this pull request 2025-12-16 09:41:45 +01:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
JoshStrobl
Labels
Clear labels   
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
wontfix

This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
BuddiesOfBudgie/budgie-desktop-services!19
No description provided.