tde-devs@chat.jabb.im < 2022/07/17 > |
[09:03] | blu.256 has joined |
[09:12] | blu.256 has left |
[09:49] | blu.256 has joined |
[10:29] | michelec2 has joined |
[10:30] | michelec2: tdebase#285 updated again |
[10:33] | michelec2: @blu.256: sorry, hadn't seen your comment on your own implementation |
[10:41] | blu.256: ok, looks a little different from what I did |
[10:42] | blu.256: let me test yours |
[10:46] | michelec2: (y) |
[10:46] | michelec2: basically I have two variables to save the user choice for Tiled and NonTiled position. Then it works as the previous version |
[10:54] | blu.256: this is approximately what I did, but still looks a little different |
[10:54] | michelec2: ok |
[10:54] | blu.256: I just built it and can finally test it |
[10:54] | blu.256: so I can tell the differences |
[10:54] | michelec2: if you think you have a better solution, feel free to share on the PR and we can merge to my changes |
[10:54] | michelec2: (y) |
[10:58] | blu.256: looks like your implementation has a few issues |
[10:59] | michelec2: ok. what are they? |
[10:59] | blu.256: let me write them down |
[10:59] | michelec2: ok |
[10:59] | Slávek has left |
[11:06] | blu.256: see comment on Gitea |
[11:06] | michelec2: (y) |
[11:06] | blu.256: I think my implementation doesnt have those issues, but there's only one way to be sure |
[11:06] | blu.256: I don |
[11:06] | blu.256: 't want to overwrite your commit in the PR |
[11:07] | blu.256: so maybe I can send a patch of my implementation? or push to a new branch? |
[11:07] | michelec2: you can push a commit on top of the same branch. If it works well, we then squash the two commits together and use "Sign-offoby" both of us |
[11:07] | michelec2: *Sign-off-by |
[11:08] | michelec2: or another way is to separate the fixes for TDE/tdebase#280 and TDE/tdebase#281 in two different PRs |
[11:08] | tde-bot: [TGW][tdebase] #281 - konqueror: "Set as Background" action on images applies wrong positioning
https://mirror.git.trinitydesktop.org/gitea/TDE/tdebase/issues/281 |
[11:08] | tde-bot: [TGW][tdebase] #280 - kdesktop: default positioning of wallpapers ignores aspect ratio
https://mirror.git.trinitydesktop.org/gitea/TDE/tdebase/issues/280 |
[11:09] | blu.256: no need to go to such lengths maybe |
[11:09] | michelec2: ok |
[11:09] | michelec2: then add a commit on top of mine and we go from there |
[11:09] | blu.256: i'll push atop the existing commits. meanwhile i'm rebuilding so that i can test my impl for these specific issues |
[11:15] | michelec2: ok |
[11:15] | blu.256: merge conflicts :( |
[11:20] | blu.256: i think i'll just force-push this commit |
[11:21] | blu.256: if needed we can reset to the previous changes by commit id |
[11:21] | michelec2: ok |
[11:22] | blu.256: just pushed, still building |
[11:29] | michelec2: ok |
[11:30] | blu.256: just finished testing, these issues do not show up |
[11:30] | blu.256: you can test it too or review the code |
[11:31] | michelec2: see comment on .h file about init of variables |
[11:31] | blu.256: ok |
[11:35] | michelec2: and further comments on && vs || |
[11:40] | michelec2: other than that is seems ok, although I have not tested |
[11:41] | blu.256: fixed |
[11:44] | blu.256: you might want to test it, and it would be good to hear feedback from Slávek too. with that resolved, I don't think I have anything else to comment on TDE/tdebase#285 at the moment |
[11:44] | tde-bot: [TGW][tdebase] #285 - Improve setting background images.
https://mirror.git.trinitydesktop.org/gitea/TDE/tdebase/pulls/285 |
[11:45] | blu.256: I should now squash the two commits into one, shouldn't I? |
[11:46] | michelec2: sure, you can squash and then I test |
[11:47] | blu.256: done :) |
[11:50] | michelec2: (y) |
[11:50] | michelec2: will build and test |
[11:51] | blu.256: ok (Y) |
[12:07] | michelec2 has left |
[12:07] | michelec2 has joined |
[12:12] | michelec2: see comment on TGW |
[12:12] | michelec2: do you see the same issue? |
[12:24] | blu.256: let me see |
[12:27] | blu.256: not sure what you mean by "opposite does not seem to happen" but I see what is the issue |
[12:27] | blu.256: only the option for the currently selected wallpaper type is stored |
[12:28] | blu.256: so if you select a tiled one only the tiled option will be stored. otherwise if you select a nontiled one that option gets stored instead |
[12:29] | blu.256: if we want to store both options we'll probably need to add them to KBackgroundSettings class |
[12:30] | michelec2: I think it is not that important |
[12:30] | michelec2: as long as it behaves ok while the dialog is open, it should be fine IMO |
[12:31] | michelec2: by "opposite side" I meant selecting a tile image, But I think I mistested that :-$ |
[12:35] | michelec2: let's see what Slavek thinks about it |
[13:13] | blu.256 has left |
[14:00] | Slávek has joined |
[15:01] | Slávek: TDE/tdebase#285 seems good |
[15:02] | tde-bot: [TGW][tdebase] #285 - Improve setting background images.
https://mirror.git.trinitydesktop.org/gitea/TDE/tdebase/pulls/285 |
[15:02] | Slávek: Guys, it was a very good collaboration again! |
[15:10] | michelec2: ok, then I will merge as is |
[15:44] | michelec2 has left |
[22:15] | Slávek has left |
[22:52] | Slávek has joined |
tde-devs@chat.jabb.im < 2022/07/17 > |