| 8 Apr 2019 |
 sophie | If you like, but there's still no garentee this is what they want. hpvb indicated to me his plan was something like this (and the PR was originally based on his code) but he hasn't looked into the details of my efforts yet. | 14:14:14 |
 ddevault | the code's going to be useful no matter what, I think | 14:14:35 |
| ddevault | moving it between abstractions isn't that hard | 14:14:43 |
| sophie | True, and understanding my PR will give you a better idea of the problem space if we need to do something different | 14:16:18 |
| sophie | Another thing someone could do to make it easier to merge is split DisplayDriver part of my PR from the Freedesktop/LinuxBSD part. You could have one PR that introduces the DisplayDriver concept and makes all the OS_* classes (including OS_X11) inherit from it (half of my first commit, and all the others), then another PR to split OS_X11 into OS_LinuxBSD and DisplayX11 (other half of my first commit). It would be more logical, but a lot of work, so I was only planning on doing that if they asked. | 14:21:49 |
| ddevault | I imagine the other way around would be easier | 14:25:08 |
| ddevault | move OS_X11 into linuxbsd, then introduce DisplayDriver | 14:25:18 |
| sophie | Could work as well. Not sure if that would be easier or not | 14:28:24 |
| ddevault | in that approach, the first commit is basically just renames, the second commit looks the same as your current patch less the renames | 14:29:09 |
| sophie | ddevault: If you could change your PR (https://github.com/godotengine/godot/pull/27266) to LinuxBSD, and removed the splitup of what is now GenericUnix and X11, that would be helpful. Basically just a strait rename of X11 to LinuxBSD, without changes to structure at all. I'd rebase on top of that. | 14:41:27 |
| ddevault | cool. I'll do that tomorrow morning | 14:42:01 |
| sophie | I'll comment there and let them know what we're planning | 14:42:35 |
| ddevault | ty | 14:44:39 |
| 9 Apr 2019 |
| ddevault | wmww: want me to rebase your patch and push a new version to your branch? | 15:17:23 |
| ddevault | err, I think on matrix you use @wmww | 15:17:32 |
| sophie | ddevault: both worked. Is your branch updated? I'll do it, since I've been in that code for days now | 16:02:40 |
| ddevault | it is, yes | 16:04:50 |
| sophie | ddevault: A few nits (I'll leave them here instead of publically reviewing): | 16:08:28 |
| sophie | You still have is_unix_or_server_arm, which might be misleading because Mac is a Unix but not included | 16:08:52 |
| sophie | get_name() returns the string "Linux/BSD". This obviosly looks nicer, but I would keep it just "LinuxBSD". If anyone tries to check it programaticlly they might forget the slash. | 16:10:35 |
| sophie | ddevault: If you want to rebase my PR on top of your stuff you can. I anticipate it will be quite a bit of work. I'll get started when I get off work (5hrs or so), so let me know if you do anything before then. | 16:16:54 |
| ddevault | wmww: ack on all counts | 16:23:48 |
| ddevault | eating lunch now but I'll probably get in and start on rebasing your patch | 16:24:00 |
| ddevault | wmww: rebased: https://github.com/status-im/godot/tree/display-abstraction-rebase | 18:55:10 |
| ddevault | also rebased against upstream master to deal with that merge conflict | 18:55:23 |
| sophie | Nice. just grabbed it and making a few tweeks before I use it to update my PR | 22:11:11 |
| sophie | ddevault: I made a few fixes to your rename branch: https://github.com/wmww/godot/commits/generic_unix | 23:17:01 |
| ddevault | thanks, let me cherry pick these | 23:17:30 |
| sophie | To be clear, I'm considering myself to have the mutex lock back on the display abstraction stuff. Since we're editing the same commits over and over we can't combine changes if you do anything to that now. | 23:18:14 |
| ddevault | updated the display-abstraction-rebase branch as well | 23:18:25 |
