Making future merges easier (Whitespace Proposal)

[Signal Linden (Bennett Goble)]

The topic of making whitespace consistent in the SL viewer codebase was brought up during the last TPVD meeting on 10/28. Unlike previous attempts/discussions about linting and style changes this is only being broached in light of a simple and automated way to merge these changes into forks.

There are two main benefits to making spacing consistent:

1. Reduction of merge conflicts - many forks have whitespace that varies from upstream. This causes unnecessary conflicts when rebasing features.

2. Improved developer experience - Developers will not have to deal with mixed-whitespace files

Both of these points are important, and the impetus is, in part, seeing all the extra noise inconsistent whitespace adds to rebasing features like puppetry onto TPVs. Having such a basic thing be so inconsistent is also, frankly, a bit embarrassing.

To demonstrate adopting these changes is not actually that hard, or world-ending, the whitespace changes have been incorporated into Firestorm and Kokua's codebase in the following repositories:

https://github.com/bennettgoble/firestorm-phoenix

https://github.com/bennettgoble/kokua

Adopting these changes took ~30 seconds on WSL after becoming familiar with the merge process, and required that the codebase be up to date with LL's master as of 11/01.

How does the merge work? Draft instructions are here: https://wiki.secondlife.com/wiki/2022_Consistent_Whitespace but, essentially, they rely on git's ignore-all-space merge strategy that ignores whitespace changes, and a simple whitespace fixing script that TPVs can use to ensure their own files are updated.

The current DRTVWR branch adopts spaces over tabs. This is done because the industry has largely resolved this debate: Microsoft, Google, GNU, and LLVM style guides all use spaces. The vast majority of open source C++ code written ([1], [2].) It would be pretty odd for us to go with tabs, even if the viewer started out that way as the process of adopting one or the other is the same.

This is still a proposal, but were we to move forward the adoption would look like this:

1. (Done) A DRTVWR is made containing only whitespace changes

2. (Done) Merge instructions written and shared

3. (WIP)  Soak time for folks to test out merges

4. (WIP)  DRTVWR is QA'd by LL

5. An official date for DRTVWR merge is shared to TPVDs and opensource-dev mailing list

5. DRTVWR merged into master

6. LL updates C++ style guide

6. TPVs merge LL master changes by following aforementioned instructions

We, collectively, should be able to handle changes like this to improve the quality of life for everyone. In addition to streamlining the CLA, moving to GitHub, increasing PR turnaround time, and entertaining the notion of bringing the linux viewer back: it's going to become a lot easier to contribute features to SL in a modern manner.

Let me or Vir know if there is any feedback. I am, personally, rather interested in seeing this through so please reach out if you/your team wants help, be it questions, or even help incorporating these changes into your codebase if this moves forward.

Second Life: Signal Linden

Discord: Signal Linden#9329

[choraz...@gmail.com]

Hi,

 

Please be aware that the presence of a Kokua branch as one of the examples here DOES NOT imply my support or endorsement of this proposal.

 

Kind regards

 

Alan

 

From: opensou...@lists.secondlife.com <opensou...@lists.secondlife.com> On Behalf Of Signal Linden (Bennett Goble)
Sent: 11 November 2022 00:58
To: OpenSource Mailing List <opensou...@lists.secondlife.com>
Subject: [opensource-dev] Making future merges easier (Whitespace Proposal)

 

<snip>

[Henri Beauchamp]

On Thu, 10 Nov 2022 16:58:05 -0800, Signal Linden (Bennett Goble) wrote:

> In addition to streamlining the CLA

Indeed: at last a CLA I should be able to sign...

> moving to GitHub

And that will be a "good move" ! :-D

> and entertaining the notion of bringing the linux viewer back

Bringing it back would indeed be a great piece of news !... Especially
seeing how much better and faster the (third party, for now) viewers
run under Linux ! :-P


As for white spaces and coding style... Well, I'm glad I forked the
Cool VL Viewer in the v1 viewer days (*) and can use my own,
personal conventions without disturbing/impairing any one else. :-P

Regards,

Henri.

(*) BTW, on the 16th of this month, the Cool VL Viewer will be 15 years
old...

[Niran]

I'd certainly be on board for tabs over spaces but as it currently stands, spaces over tabs is not something i will support. Wherever i go, whatever code i look into it is almost always with tabs and i much prefer it that way. Not to mention that whitespace merges are at most a slight inconvenience, yes they show up but merge/compares usually have a toggle to ignore them.

I'd rather see them decide on whether something like single line code should be legal rather than this. Much much worse seeing something like

if(){

stuff;

}

rather than 

if()

{

 stuff;

}

--
Archives of earlier incarnations of this list are at https://list-archives.secondlife.com
---
You received this message because you are subscribed to the Google Groups "opensource-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to opensource-de...@lists.secondlife.com.
To view this discussion on the web visit https://groups.google.com/a/lists.secondlife.com/d/msgid/opensource-dev/20221111230429.cc94db611c484b90fd4b9702%40free.fr.

[Ricky]

I'm a fan of tabs. However I work in a space, admittedly mostly Typescript, that uses 4 space indentation. IMO style guides are the purview of the project admins. As long as an automated tool takes care of it, meh, whatever. If I really cared I could set up some git hooks to convert, but I've not seen the need.

Ditto for inlining or not the opening block statement brace: as long as the placement is automatically controlled, I don't care. Personally, coming from the broken mind of someone who was trained in BASIC, the `if () {` syntax is easier to read. But again, as long as the rules are automatically enforced, and that preferably on save, it doesn't matter to me.

Ricky,

Cron Stardust

To view this discussion on the web visit https://groups.google.com/a/lists.secondlife.com/d/msgid/opensource-dev/CAOVTa-S-XcvwpT5g8XZq87Z1R4Dtoo7ax4dS6MJhLzE_7%2BJPHw%40mail.gmail.com.

[Argent]

LOL. Back in the 90s I was the nerd voluntold to sit on the C coding standards committee at work and write up the draft. There was a huge debate over where to put braces, so I just put in our standard that it should line up with the if statement or whatever. Both sides interpreted that as agreeing with them and we could move on. Luckily we didn't have any indented-braces fans.

To view this discussion on the web visit https://groups.google.com/a/lists.secondlife.com/d/msgid/opensource-dev/CAFsNfZKRKbZvLLHp1_iCYSD-7KBSc750KtYcnTWcV%2BFUiruA-A%40mail.gmail.com.