Improve contribution guidelines

[selfhoster1312]

Following discussion in matterbridge-org/matterbridge#14

we really need more detailed contribution guidelines and how to make our lives easier

[selfhoster1312]

Existing guidelines (very small): https://github.com/matterbridge-org/matterbridge/tree/master/docs/development

[lil5]

That readme should be moved to /DEVELOPMENT.md

[poVoq]

Matterbridge by its very nature, and even more so in this community maintained fork without the previous BDFL is going to attract a range of people with widely different coding standards and expectations.

Automated checks and tools are a way to keep things somewhat standartized for better maintainability, but given that this is a fairly new project we are still figuring out the best balance between this and not alienating new contributors.

Your critisism about excessive linting is not without reason and I agree that this can probably be tuned down a bit. On the other hand, functionally seperating PRs and especially not doing changes in the CI or major version bumps in totally unrelated PRs is a pretty reasonable requirement as well.

I hope you will stick with us while we figure out these things ourselves and in general I hope it is clear that @selfhoster1312 attempts to improve your contributions to be more in line with our expectations is not in any way an attack on you or your contributions.

[lil5]

I agree entirely, I should have moved the lint contributions/ golang updates, to a separate issue/pr.

My gripe with @selfhoster1312 's comments stems for the issue they take with me making a commit "Add documentation" and "more docs" and expecting me to follow a commit style, I have zero interest in following.

When I work on a project's feature branch I don't see commits as a sub task to be amended to, where the git history is mailable. I see it as an immutable history of my progression on working on the PR, this creates a messy branch full of alterations. I don't want to be scared of creating two commits to change the documentation because it's not part of this project's policy.

[jugendhacker]

When I work on a project's feature branch I don't see commits as a sub task to be amended to, where the git history is mailable. I see it as an immutable history of my progression on working on the PR, this creates a messy branch full of alterations. I don't want to be scared of creating two commits to change the documentation because it's not part of this project's policy.

This will lead to a very noisy commit history on master though. So IMHO PRs should either be squashed on merge (which might lead to quite big commits that are not really "atomic") or contributors should be expected to submit a "clean" commit history (e.g. commits that ammend another commit should be squashed) before getting their PR merged

[lil5]

@jugendhacker The way I see things being done on most GitHub projects are to squash and merge so that the contributor doesn't have to think about it, and it's up to the project maintainers to decide how the main commit history looks like.

[lil5]

If you're handling multiple things in a single pr it's a bad pr in the first place imo.

[jugendhacker]

@jugendhacker The way I see things being done on most GitHub projects are to squash and merge so that the contributor doesn't have to think about it, and it's up to the project maintainers to decide how the main commit history looks like.

In my experience this really depends on the project. And traditionally git is very much used in the way that the contributor determines commit messages etc when used with the original mail based workflow

[lil5]

@jugendhacker you are confusing projects with a set team of paid experts and open source contributors, with fly by pull requests.

[selfhoster1312]

I'm not sure why we had so many different PRs for the same features:

• discord proxy: Proxy support for Discord matterbridge#109 Proxy support for Discord matterbridge#108 Proxy support for Discord matterbridge#107 Proxy support for Discord matterbridge#48
• matrix E2EE: Matrix e2ee matterbridge#83 add matrix end to end encryption support matterbridge#85 add matrix end to end encryption support matterbridge#94 matrix: add e2ee support matterbridge#97 matrix: add e2ee support matterbridge#98 matrix: add e2ee support matterbridge#100

I think it's good to close a PR and open a new one when you have a significantly different approach to the problem and want to "archive" the previous approach (see matterbridge-org/matterbridge#2 vs matterbridge-org/matterbridge#3, for example), but i'm not sure that's what's happening here. @josephcrowell @Wohlstand, is there something wrong we can fix so that you don't have to open so many PRs? Is the problem that you're afraid to git push --force to your existing branch? Should we document something better? Feedback welcome!

I'm not sure it's exactly a problem to have different PRs for the same feature, but then it's harder to follow the discussion/review (see for example discussion in matterbridge-org/matterbridge#48 then matterbridge-org/matterbridge#109 about the new http_proxy settings and HTTP helpers), so unless i'm missing something i think we should aim to have only one PR per feature. Does everybody agree or am i missing something?

[Wohlstand]

• discord proxy:

They all are mine, and they all are just same single PR that implements the feature. I had to recreated PR multiple times during attempts to resolve the CI problem. The initial and true PR is #48 from which it began, and PR #109 is my remake because I was unable to open #48 again.

So, technically, it's only one sigle PR. These are closed, are trash that stinks. Broken and invalid.

The reason why I caused that - it was a false advice that CI problem might be solved by remaking new PR with different name, and that was a mistake, that didn't solved problem. And even more I ruined my initial PR by closing it, and after force-pushing it being closed, I no longer can open it back, and therefore I remade a fourth.

I am fine to force-push, just to squash, reorder, or adjust some, but the case that caused me produce many PRs for the same code, is a very special.

[poVoq]

According to @josephcrowell there is a Github actions specific issue that when you make changes to the CI it somehow breaks older PRs depending on these changes. I don't know the specifics, but the many closed PRs were related to trying to resolve that issue.

[selfhoster1312]

the case that caused me produce many PRs for the same code, is a very special.

Good! Don't hesitate to ask on IRC/XMPP if you ever have questions about the CI or something else :)

when you make changes to the CI it somehow breaks older PRs depending on these changes

Unless i'm mistaken, we should first merge the PR editing the CI, then you can force push to your other PR branches and it will trigger a new CI build. Is that a reasonable solution or am i missing something? (if that was indeed the problem, we should indeed document that in the contribution guidelines)