Add xdp prefixes and move portal implementations to new directory

[swick]

This renames a bunch of classes, enums and functions to have the XDP prefix. The portal implementations are moved into the portals subdir. Sources for helper binaries are moved to the new helpers subdir.

A lot of code movement but no functional changes.

[swick]

I know the diff looks daunting but this really just needs an ack on the decisions I've made here, namely:

• do we want to move everything to the xdp prefix?
• do we want to move the portal implementations to their own directory?
• do we want to move the helpers to their own directory?
• is it worth to do all of that and also require everyone to rebase on top of it?

@GeorgesStavracas @jadahl

[jadahl]

Proper namespacing is important when doing shared libraries. I'm a bit less convinced e.g. Session needs to become XdpSession in xdg-desktop-portal though, I have never felt the need to add it in the past, and in that particular case it also departs from the convention of other types corresponding to APIs (i.e. no xdp prefixing). Same applies to Request.

The moving around and renaming files I see the point, but also there are not that many files, so it doesn't get us that much, and makes git archeology a bit more annoying.

So, overall, I think the structure we have already is good enough, and this doesn't improve things enough to make it worth it.

No strong opinions though.

[swick]

Not having a prefix means we can't use the G_DECLARE_TYPE macros which is the main motivation for me. It unlocks a bunch of other cleanups, having a proper split between private and public API etc.

[matthiasclasen]

Being able to use boilerplate-reducing gobject macros is a good argument, indeed.

[swick]

I'm willing to drop the last two commits which move around files to new directories if that helps to get this moving.

[TingPing]

It creates a bit of churn for minor niceties but I'd also say I'm fairly neutral on it.

I'm not sure how many neutrals equal a positive though :P

Needs a rebase at the least.

[swick]

It pretty much always needs a rebase when something changes so I'd like to avoid doing the work until a decision has been made.

[TingPing]

I'll merge it this weekend if nobody disagrees.

[GeorgesStavracas]

I'll merge it this weekend if nobody disagrees.

Please don't merge it. Neither me nor @jadahl have re-reviewed it nor discussed it again.

[swick]

Rebased

[TingPing]

Please don't merge it.

OK. I was just going off of his "No strong opinions though."

[swick]

No one has strong opinions and I have cleanups planned on top of this so is there really a good reason to not just go ahead with it?

[GeorgesStavracas]

No one has strong opinions and I have cleanups planned on top of this so is there really a good reason to not just go ahead with it?

In my opinion this change is too big for the "no objection" policy, so it needs more than a lack of objections, it needs explicit agreement.

[swick]

This has no functional changes whatsoever. Categorizing it as big because it touches a lot of code isn't helpful. We also just merged the python formatting changes without too much scrutiny.

This PR is quite annoying because you claim that this needs explicit agreement while also not reviewing it. There is literally nothing I can do on my end other than trying to convince you to take the 10 minutes to go through the changes.

[GeorgesStavracas]

This has no functional changes whatsoever. Categorizing it as big because it touches a lot of code isn't helpful.

As unhelpful as it may sound, I think a 99-file thousand-line-diff PR can be called "big" by any practical standards. It's also mixing different changes in a single PR (changing namespaces and moving files).

We also just merged the python formatting changes without too much scrutiny.

The Python tests were merged by @TingPing without any prior coordination, nor the usual approval by whot. I did not agree with that but I respect the decision to merge it. If you think merging these Python formatting changes without a thorough review was a mistake, then I think we should revert it, and reopen the PR.

This PR is quite annoying [...]

I think we can agree on this.

This PR is large, so much so that everything that is merged conflicts with it, which might indicate that this may be too big in the first place.

Generally it is a welcomed courtesy to discuss changes like this before opening a PR. Because it is a big change, it'll accumulate conflicts the more it stays open. The way to have merged this fast was to have everyone onboard before opening it.

because you claim that this needs explicit agreement while also not reviewing it.

Be patient. You could have chosen a path where things could have been faster, but instead chose a lengthier one, and that's the consequence.

(I'd also appreciate if you could stop with the implicit accusations here.)

There is literally nothing I can do on my end other than trying to convince you to take the 10 minutes to go through the changes.

That's not only very hyperbolic (it takes a lot more than 10 minutes to review something like this), but factually there are many things you could and still can do: you could have coordinated before opening the PR; you can stop pressuring people to accept your changes without scrutiny; etc. Shouting and pressuring can only get you so far, but at some point you'll need to learn how to work productively with others.

---

Actually, as I'm typing this, I realized that not only I think this PR is unreviewable, the whole process was messed up. So I'm going to close it, and let you do things in a more appropriate and respectful way. Please try again, and do better.

[swick]

• At no point did I pressure anyone into accepting any changes without scrutiny
• I didn't shout
• I did ask beforehand if this change would be acceptable and didn't get a clear yes nor a clear no
• I asked multiple times for feedback so we can figure out how to move forward
• I don't think automated formatting needs any kind of scrutiny and implying that it does is a really hot take
• Renaming things that are used throughout the codebase necessarily introduces conflicts
• I split the changes up into small, single purpose commits, each of which is basically just a s/old/new
• I do work productively with others

The only tractable thing you said is that splitting up the PR would help reviewing. I asked before if it would help but again, didn't receive an answer.

All of this is nothing short of insulting and I take it exactly as that. On another PR you also escalated to situation because I said a combination of flags is "weird". You clearly have personal issues with me and are abusing your maintainer role here. Instead of blaming me for everything all the time, consider how you're interacting with me.

And all of that for a PR that is a trivial search and replace...

[TingPing]

The Python tests were merged by @TingPing without any prior coordination, nor the usual approval by whot. I did not agree with that but I respect the decision to merge it. If you think merging these Python formatting changes without a thorough review was a mistake, then I think we should revert it, and reopen the PR.

FWIW I merged one PR approved by whot and the other one had no logic changes and was approved by whot immediately after it was merged anyway.

All I wanted to do was lower the burden of other reviewers by doing a few myself.

[swick]

All I wanted to do was lower the burden of other reviewers by doing a few myself.

And I appreciate that. I don't see how this would have required any more attention and I feel like Georges really is just trying to put everything into a bad light here.

[swick]

Anyway, I opened a new PR which drops the commits which move things around. I would appreciate if we could all calm down, stop escalating and making things personal. We all just want to make things better here after all...