Discussing the maintainership

[marvinroger]

Hi everyone,

As @davidgraeff and @ThomDietrich pointed out on #108 (comment):

Guys, if a PR simple like this takes 2 weeks there is something seriously going wrong with the maintainership.

I completely agree with that. I have to admit that I never expected Homie to become anything more than a personal project. Despite the fact that I'm happy that it's gained traction, I quickly became overwhelmed about the project. I just don't have as much time as I used to have when I first started the project. That's why Homie is not a personal project anymore; it belongs to the Homie community, which brought a lot to it.

Once again, thanks to everyone who contributed, one way or another, to the Homie projects. 😉

Anyway, @davidgraeff proposed the following about the maintainership of Homie:

A structured way on how this convention should be evolved would help. Like:

• Is a majority of active maintainers required OR a is peer review sufficient?
• How long is the voting process for a feature. (Close issues older than that amount of time as "Invalid" or "Rejected")
• How do you determine if a maintainer is active?

I propose the following:

• A new discussion/feature can be raised by a GitHub Issue or a Pull-Request. A Pull Request is preferred, as it allows to talk about a specific idea more easily.
• The discussion period for a new feature lasts for a maximum of 3 weeks. Everyone involved can apply for a longer time frame if required. 3 weeks no progress means "Rejected" automatically.
• Active maintainers list themselves in a MAINTAINER.md file. Everyone in that file is required to response within 3 weeks to an Issue. A maintainer can remove/add himself at any time to the mentioned file if workload/holiday will prevent him participating in a foreseeable future.
• All active maintainers MUST agree on a new feature for it to be added.

Followed by the following answers: #108 (comment) #108 (comment)

I'm moving the discussion over here, as it's a real issue that deserves its own space.

[marvinroger]

My input:

A new discussion/feature can be raised by a GitHub Issue or a Pull-Request. A Pull Request is preferred, as it allows to talk about a specific idea more easily.

I think we all agree about this.

The discussion period for a new feature lasts for a maximum of 3 weeks. Everyone involved can apply for a longer time frame if required. 3 weeks no progress means "Rejected" automatically.

A lot of projects adopted this, and I think housekeeping is a good thing, as it avoids having a lot of open issues that very few people care about. Only the most important things would be discussed.

Moreover, if a ticket gets automatically closed, it does not mean that it's closed forever. The OP can open it back anytime.

Active maintainers list themselves in a MAINTAINER.md file. Everyone in that file is required to response within 3 weeks to an Issue. A maintainer can remove/add himself at any time to the mentioned file if workload/holiday will prevent him participating in a foreseeable future.

Yes, definitely. 👍
But...

All active maintainers MUST agree on a new feature for it to be added.

I like @ThomDietrich approach:

I think we should follow the four-eyes principle. A PR is approved by one maintainer and merged by another. The merging one can be the creator of the PR. Before the merge everyone can raise critique and for important changes other users and/or maintainers should be mentioned to request their opinion. An exception is the trivial case (spelling error, ...) in which a single maintainer is sufficient.

[ThomDietrich]

Hey @marvinroger, happy to see you take part in this conversation. I am now also in a new phase of my live and am still struggling to find free time to contribute to the OS world.

Regarding the four eyes principle: I think this is very much sufficient, I use it in multiple projects AND it is directly supported by GitHub as a feature in the repository settings.

Regarding "housekeeping": After consideration I can also find my peace with this. In order to enforce this rule we should definitely use an according tool. See: https://github.com/probot/stale

@marvinroger you missed one comment of mine: I suggest @davidgraeff as a contributor of this repository. All good. Already happened and I didn't realize. @davidgraeff happy to have you!
Additionally I want to suggest to add @lorenwest and a few other users to a read-only team in the organization. This will add the ability to mention them in comments etc

[marvinroger]

Wish you the best in your new phase of life, @ThomDietrich 😉

I never worked with teams. I created the @homieiot/involved team. We should think of a "team tree", or whatever it is called, that defines who have the right to do what.

[davidgraeff]

@marvinroger Could you please lock the master branch from receiving pushes in the settings at some point? At the moment all PRs are going against master, but they should go into a "develop" branch if I understood correctly to accumulate for a manual release.

[marvinroger]

@davidgraeff AFAIK, we can't "lock" the master branch. The only thing available is this:

I am not sure this disallows the creation of a PR request. The only thing we can do is set the base branch to develop. But then, people landing on the repo would still see the 3.x.x.

So, @davidgraeff's suggestion to explain things in the README.md file, and to put the convention in a separate CONVENTION.md file might help. The master built branch could still only contain the convention in the README.md. What do you think?

[davidgraeff]

Go to "Branches" -> Branch protection rule -> Add new rule and enable "Require pull request reviews before merging" ^^

[davidgraeff]

There should be no file renaming involved while merging from "develop" into "master".

[marvinroger]

I'm confused. How does "Require pull request reviews before merging" help?

When enabled, all commits must be made to a non-protected branch and submitted via a pull request with the required number of approving reviews and no changes requested before it can be merged into a branch that matches this rule.

The thing is, only the CI will push to master. Nobody should open a PR against master.

There should be no file renaming involved while merging from "develop" into "master".

Same thing, it's not about merging develop into master. I'd like the master to be kind of a gh-pages, but with a single preprocessed README.md file instead of a website. And this commit is the one that will be tagged as v3.1.0, for example.

[davidgraeff]

This makes the entire process fully dependent on a tool. And if the tool developer leaves the project at some time, you get the point. Who will trigger the tool is my concern, because GitHub does not give us any interface.

An idea would be: The tool pushes to a "latest" branch or so whenever something is pushed to "develop". A tag with a computed version is automatically attached to "latest". And humans in the end decide when to merge "latest" into master.

So much effort to get semver right :D

[marvinroger]

On a tool that is part of the repository. The release tool would be stored into a .ci directory. If the developer leaves the project, the tool (script) is still available.

How to trigger a release? We already have [skip ci] available, we could easily add a [release] to commit messages (not on the first line, to keep it clean, but in the "body"), as part of the squash commit.

Therefore, everything is clean and tidy, and, as long as this behavior is documented, it is clear how to release a new version.

Not having to deal manually with versions and releases, is a huge step forward, I think. ;)

[davidgraeff]

You need an empty commit for triggering the release. That's your plan, ok.

There was not a single commit in the homie repo for about half a year. The convention works, and there are not many open questions left. I'm just asking to keep a balance between the effort (tool creation, maintenance) and outcome.

Just to summarize: The tool would

• go through all commits, compute if major, minor, patch version increases are required
• create a tag
• push to master,

right?

[timpur]

Just something id like to see more personally, is us "maintainers" also discuss more on slack about where we all want to see homie to go and if any "debates" about issues can be carried out on slack and the result committed on the issue. I like discussing things, just some times i dont think it belongs on an issue ticket, because then the ticket gets bloated and confusing and takes weeks to sort out. Faster coms can help resolve a discussion.

[marvinroger]

@davidgraeff

You need an empty commit for triggering the release.

Not necessarily, we could decide before merging the PR whether or not the merge should trigger a release. If we decide it should, we add a [release] in the body of the squash commit.

I'm just asking to keep a balance between the effort (tool creation, maintenance) and outcome.

The script is already done. 😉
I have to add a few more things, but it's almost ready.

The tool would indeed:

• Go through all commits, compute if major, minor, patch version increases are required. Continue only if one of the commits contains a [release] in the body.
• Build the convention (currently, it's only a matter of replacing the 3.x.x with the actual version)
• Push to master
• GitHub release/tag the resulting commit

@timpur

I honestly don't have time to follow real-time conversations. GitHub issues are ideal for me.

[timpur]

@marvinroger true, if that what everyone thinks is best :)

[Thalhammer]

• Active maintainers list themselves in a MAINTAINER.md file.

I'm not sure if this is really a good idea since it would result in a lot of (basically) useless commits. I do agree that we should keep track of who is active or not, but I don't think a file inside the repo is the right way to do this, It just feels wrong.

Although the convention didn't change in the latest time, there are still a lot of questions which have not yet been addressed, A small subset:

• OTA (Add more hardware information to simplify OTA #83 )
• $stats (Homie-Statistical-Values #102 )
• Some things are required for no real reason but might cause problems (e.g. $mac)

That said, in case there is a need for additional maintainers: I have quite some free time to spend 😄

Regarding $mac: There are a lot of cases where a mac makes no sense. MAC addresses are only available in IEEE 802 networks (WLAN, LAN) and while many sensors will indeed use those, I already have 2 different examples where I simply have no mac. And that's just in my deployments. Neither TCP/IP nor MQTT require a MAC, so homie, which is even higher in the stack should not require one. I don't say we should ban it, just that it should not be a required attribute (and IP as well, but that's not such a big problem).

// EDIT: Ignore everything about mac, I didn't see #120 before this.

[davidgraeff]

The website script is capable of collecting specifications from different repositories. My idea is to specify OTA in a different document, requiring Homie Core 3.0 as base specification. For $stats: I have no idea on what to do with those topics. I made them optional in #120.

[Thalhammer]

Yeah I agree that OTA should be defined in a separate document, but there should be some sort of list that contains all additionaly supported subsets (including there versions).
I though this was already discussed in a different issue, but I can't find it right now.

[davidgraeff]

The website lists all found specifications on the left navigation bar. Do we need a list somewhere else, too?

[Thalhammer]

What I meant is that the device should announce a list of subspecifications it supports to allow for simple discovery. Something like homie/device/$subsets => ota(1.0.0)

[davidgraeff]

Nah. I'd say each document defines it's own way of how discovery works like homie/device/$homie-ota

[Thalhammer]

Wouldn't this cause problems if a device gets replaced by one that doesn't support this feature ?
Controllers would still assume it's supported since the topic is retained, but the device has no idea about it.

[davidgraeff]

That is true for homie itself. There is no challenge-response, last-time-updated or heart-beat process to verify that a device still exists. We could make a last-will mandatory that undefines those topics.

[davidgraeff]

@Thalhammer I think this discussion might belong to a separate issue, doesn't it? :)

[davidgraeff]

I agree that a file MAINTAINERS.md might not be the solution.

I have added a list of active maintainers to the website in the section "Get Involved". I invite everyone with interest to make a PR and add himself to the list. @marvinroger need to approve those PRs.

See: https://homieiot.github.io/Get_Involved/#active-maintainers

I'm closing this issue now as resolved. If not resolved, please reopen.