Regarding patch acceptance policies

Commit access to the repository is granted to people who are trusted to have good understanding of:

  • Our coding style
  • The way we discuss things together and reach decisions in a peaceful way
  • The shared vision we have of what Haiku should be and the direction we should work on

This is granted through public proposals to the haiku-development mailing list after one of the existing commiters sends a proposal to add someone.

While X512 has done a lot of work so far, at least the first condition is not met, as patches consistently have several coding style problems, with apparently not a lot of motivation to go back on older commxts and fix the problems. There was also communication difficulties as they didn’t use IRC and mailing lists, which is one of the places where a lot of discussions happen. I think this latter part is now fixed, with the use of a Matrix bridge for IRC access.

As long as at least the coding style aspect is not resolved, the process will not go further.

Both of these problems resulted in some frustration for everyone. Communication issues result in misunderstanding. Code that does not follow the coding style requires a lot of work to clean up, and there is pressure on the commiters to get things merged when it blocks a big new feature like the risc-v pomt. Eventually we decided to merge the code anyway. Kallisti5 fixed several coding style problems in the platform-shared code, and the risc-v specific code is in a “we need to clean this up later” state.

This approach results in improvements in the short term (we get a risc-v port!) but it adds technical debt (code that we need to rework later). And people who do the clean up may introduce bugs (as with any change to the code). The resulting perception is that X512 is the cool developer working on new features, and whoever decides to do the cleanup appears to be breaking things (as you complained earlier).

So that’s why no one has proposed X512 for commit access yet. Note that other people regularly get commit access (I think the most r cent one was madmax, and before that Kyle Ambroff-Kao). Their patches were not as large, but were better in following the coding guidelines. We value the quality of changes over the quantity.

It is understandable that the work on a new architecture port requires a lot of experimentation and it is not possible to do it cleanly in the first try. You have to try things, make mistakes and go back. And it’s not always obvious how to get that ready for code review. I’m sure that now with the first set of patches merged, we have a better base to work on and if X512 continues work from there, we can consider granting them commxt access soon when we feel that the conditions are met.

9 Likes

I currently see no benefits in commit access or Gerrit privileges. It just increase probability someone hold a grudge against me. More privileges cause more troubles.

6 Likes

Some patches are based on Mini OS code with different code style (CamelCase everywhere, no underscores, same indent level for opening and closing elements, operators go to the line end, not beginning as in Haiku, “*” come to the name, not type). I personally prefer that style and I use it in my projects. Mostly the same style is also used in Oberon projects.

It may be confusing am I writing code for my personal projects or for Haiku code.

It may be easier for me to write working code first and then adapt code style.

13 Likes

It’s never easy to pick someone else’s style. I had some trouble with it when I started contributing to Haiku as well. Now I’m accustomed to the Haiku style and I started using it at work as well to keep things easier for myself. But not everyone can do that.

Thatws one of the reasons we have two GSoC students working on haiku-format so we can have a more automated way to handle this. I think it will reduce problems at least on this aspect

8 Likes

Regarding coding style, at work I’m the author of my employers coding style documents and I’m the chief architect, and I’ve never rejected a patch from collegues or juniors for not adhering to the style spec. I’ll occassionally remind them about it, they can clearly see a difference between their class style and 1000 other classes with the recommended style, but at the end of the day, I always accept the patch even though it doesn’t adhere to the style. Resources are better spent on the next commercial thing rather than a new investment in time to refactor style.

With open source, where free developer time is limited, I would prefer to accept working code by a senior engineerer regardless of style, and then let the juniors volunteer to modify the style. As Adrien has pointed out in an earlier comment, a GSOC student is working on a styling tool, so style related issues should be resolved in time automatically.

Just a pragmatic personal opinion.

7 Likes

If there are problems within the community, it must be allowed to be discussed. Some developers have tried their best to sweep this discussion under the rug by locking the thread created for it and directing us to use alternative, non visible channels. It’s censorship and will not solve the problem.

Discussing the behavior of an individual is not a personal attack.

I am happy to “censor” things here if it’s needed. This forum is hosted by the Haiku project and we decide what the appropriate behavior is here. If youare not happy about that, please find some other place or host your own forum so you can use your freedom of speech there.

In particular when a thread gets dozens of messages from people who have no clue about the technical issue being discussed (again, this was a discussion about wether a specific bootloader type should be used in haiku sources or not), I see no point in keeping it open. It was not interesting from the technical point of view, and it was alsonot interesting from the governance point of view because there was no constructive discussion on that side either. The discussion was better this time on that area from waddlesplash doing his best to explain how he does things, and there were other developers explaining that they disagree with it. That part is constructive. But random community members demanding that a developer is banned in public? That’s not constructive at all and only serves to stir up the controversy and prevent cool and calm discussions. So, please stop.

If you have problems with specific people, you can contact them privately instead of stirring hate and asking the mob to lynch them here. If private discussions don’t go anywhere, you can use the forum features to signal inappropriate posts to the moderators, or contact them privately as explained in our “policies” page.

There is no point

25 Likes

Would you accept patches without documentation with just “fix riscv” as commit message?

1 Like

We davon germany: “Der Ton macht die Musik” (The sound makes the music), in other words, it depends on how you criticize something. the post contained personal, aggressive comments.

I could understand this step. the forum operator is responsible for its content and has the duty to intervene when things go too far.

5 Likes

If you would like to have a hand in developing Haiku, please step up and start submitting patches. The process is pretty clear.

Community feedback on features and design decisions is helpful (and needed!). See Sending data to 3rd party (regarding Gerrit #1875) for a good example of this.

Community feedback to developers (working for free) telling them how to develop (while you are not helping develop) is not as useful nor wanted.

I personally value X512’s feedback on the way things are.
I don’t value external perspectives as much.

That seems a little harsh, I don’t see why third party devs can’t offer experiences in contrast to how we develop, it’s also good to know other styles, and problems with them.

Not every dev will want to develop the OS itself, that doesn’t make their programming knowledge (or project managing knowledge) less valueable.

2 Likes

Sure it does. Every development ecosystem is different and special. You don’t make changes overnight based on someone from the internet’s whims. You make process changes based on feedback of new developers mixed with the knowledge of old developers.

I’m pretty sure if I rolled up to Microsoft HQ and started yelling at their manager that the feng-shui of the curly braces is wrong i’d be escorted off the property… No matter how much experience I have.

I’m sorry for being harsh, but honestly… we need more developers. We get a lot of input, but few stepping up. It’s extremely frustrating.

https://bikeshed.com is alive and well

1 Like

Especially when the issue being discussed at hand is more of a governance experience. External feedback of how other projects handle things are basically on the same playing field as historical facts and failures.

They’re barely ever useful if the person issuing the feedback, no matter the intentions (whether you’re a user, an actually concerned community member or acting like the dictator of Haiku to contribute to chaos), cannot apply an entirely different situation and adjust it to the context of another project or group of people exhibiting similar behaviors through critical thinking. As in, if you can observe a difference, but cannot articulate the advantages and the disadvantages that actually apply in the different situation, it’s not that useful. For instance, sure, there may be a company that puts speed and efficiency over doing things differently than us, but

  • we’re not bound to deadlines
  • we’re not (usually) paying people by the hour
  • our “product” has a much larger lifecycle than, say, 3-5 years
  • we don’t even know the product and who the target audience is, so I can’t even compare that

Keep in mind that I also made a lot of presumptions here, many of which are probably highly inaccurate. Without the information that I basically just presumed in order to make the argument, this information is less useful. The usefulness also decreases if a person is not familiar with the goals, with the ability to technically apply a solution or articulate how such a solution would be laid out in the context of Haiku and its internal procedures, so on and so forth. This information ends up being harmful when people info-dump all of their pet peeves, make a huge generalization out of a specific situation, and then put the situation inside of that “box” – it’s anything but productive, just go to Hacker News.

At work where I’m architect for a team, we have systematic code review and patches regularly are reworked because of coding style problems as well as other issues. Most, but not all, of the coding style, is handled by clang-format being run automatically on all merge requests there.

The coding style we have goes with a rationale (explaining why we picked a specific naming convention, etc) and the whole team agrees on it (and can request changes or clarifications when they don’t agree on something)

There are also rules for formatting commits messages, not breaking unit tests, and running a static analyzer on the code to catch various problems. It also watches a few other metrics like the comment:code ratio (and we have contractual agreements with our customers about these).

Compared to this, the Haiku code review process is quite relaxed. We have no unit tests (or at least, we never run them). We don’t have any requirements to write documentation. At the moment, our only hope to keep the code maintainable is that at least the code itself is readable by everyone, and that is a lot easier with a consistent style. Overall, it’s just a question of different teams, different priorities. The fact that Haiku is our week-end/evening project means we can work away of any time-pressure to ship the next feature and enjoy taking the time to write nice and clean code. I have other projects for myself when I want to quickly write horrible hacky code and feel that I got a lot of things done over the week-end too, but I feel the Haiku git repository is not the right place for that.

Anyways, the situation with an opensource project is widely different from the situation in a company. The reason for this is that a significant part of the patches we getr are “drive-by” patches. People will submit one patch to fix a particular bug they were getting, and move on to something else. The maintenance burden on keeping that code working then falls on the “permanent” development team. So we need to make sure that we understand the code and that it is up to our quality standards.

Also, I did mention that in the case of the RISC-V port, we did decide to merge the code and clean it later, which is indeed a perfectly valid approach. The other part where we are more strict about this, is granting commit access to people also allows them to review and merge other’s patches. And to allow that, we need a higher level of trust in their ability to understand the coding guidelines and the architecture of the OS. You can consider that the annoying review process before someone gets there is a kind of training for their future role as a reviewer.

3 Likes

I think Zenja was clear about it being a personal opinion on how they would run their own project, and not demanding that Haiku changes its ways? Which seems fine to me. Let’s at least listen to other’s opinions, even if we don’t implement all suggestions.

4 Likes

To be precise, I actually cleaned the easy to fix style issues before merging. I dedicated to clean up the more difficult stuff later. (and already started some of it)

This is another reason for my gruff reply above to Zenja. We’re doing the actual work.

3 Likes

What I’ve learned recently as a result of my participation in other projects and in Haiku itself is that the community/users also can have very good things to say and should be listened to (that way most people can be happy, even if you can’t please everyone), but things should be explained to everyone properly and not in a “let’s drag a conversation to the public or give them a lackluster amount of information and expect to get accurate responses that will assist in the way we develop things”, btw.

Is this discussion a glimpse of the Soul of Haiku?
A soul that is playful, erratic and cruel at the same time?
Devs: just continue like this because it is refreshing to witness how the Soul of Haiku keeps growing into a higher level by understanding itself better as a whole!!
Maybe it really reflects the complexity-simplicity of its own name: HAIKU?

The fact that Haiku has come this far while maintaining simplicity means they’re doing something right.
Trust me: Once you start turning a blind eye to code that does not meet the quality or semantic standards, the code becomes a hell to maintain.
In the Netherlands we have a saying: “Zachte heelmeesters maken stinkende wonden.” It means something like “If you are too nice, you’re going to make a mess in the long run.”

Haiku is not going to become the next Windows or macOS anytime soon. If that’s your wish, find a big budget somewhere, hire some developers and fork the project. Otherwise, be patient, the developers have known what they were doing for the past 20 years and it’s gotten us this far.
Haiku is on the verge of becoming (really) suitable daily use, though. For me, minimal 3D acceleration and Virtualization are the last things withholding me from installing it as my main OS. (others may have different wishes, like a better web browser or VPN) I love Haiku, it’s a very natural way of using my PC that I can’t find in Windows or macOS. But although it works so well for me, I have no illusion that it’s going to suddenly become the third big desktop system.

The current version is not promising to be stable, stop expecting it, report bugs if you find any or help the development if you can. The Beta 3 is an excellent preview of what it can become in the next couple of years but it’s not meant to be a finished product. That’s why the Beta label is totally appropriate.

13 Likes