Gitea contributor feedback

Dear @wxiaoguang,

I would like to let you know how I’m feeling, as a Gitea contributor, regarding our interactions during the review of the store the foreign ID of issues during migration by realaravinth · Pull Request #18446 · go-gitea/gitea · GitHub pull request and the impact it had on my work and my motivation.

It started when you required changes that are non blockers after the pull request was approved. I took the time to discuss them with you in detail in the chat room and you agreed the changes were not essential and the PR could be merged. A few days later you reverted your opinion and pushed a very large refactor on top of the pull request without asking if I was ok with it. Then you raised questions about a possible bug.

These are the facts and they had a significant negative impact on me, let me explain why.

I do my best to provide quality work and to promptly address review comments to the satisfaction of the reviewer. When my work is accepted with two approvals, this is rewarding and, I believe, within the range of what is expected from any Gitea contributor. When you blocked the merge after I thought the work was accepted, I was disappointed, which is not a nice feeling: the work that I thought to be complete is being questioned. It would be fine if the problems being raised were missed by the previous reviewers. But in this case the changes you required were, by your own admission, non essential and as a result I was also frustrated in addition to being disappointed. I asked myself: why not address this refactor after the PR is merged? I did not nothing wrong and my work is blocked, why am I being frustrated? I did not express my feelings and simply commented on the conclusion of our conversation in the pull request.

When you pushed a refactor of your own on top of my PR a few days later it was a shock: you forced me to accept your changes. I was also very confused as to what I should do next. Are you going to change your opinion again? Are you going to push more commits on top of the PR without discussing them with me? Should I wait until you try to fix the bug you discovered yourself?

In a nutshell, within a few days your actions as a reviewer of this PR changed me from being a happy Gitea contributor eager to see a PR merged into a disappointed, frustrated, shocked and confused Gitea contributor. I’m however absolutely certain this was not your intention and you are most probably very surprised to read this message. I also believe you are one of the very best Gitea community members and I look forward to keep working with you. It is my hope that by clearly explaining how I felt during this review we will have more satisfying interactions in the future.

Cheers

2 Likes

More contexts in the PR: https://github.com/go-gitea/gitea/pull/18446#issuecomment-1035817949

I have said Neutral , it means that I didn’t against and didn’t approve. Since I have to approve to remove the Request Changes state, I reviewed the code line by line, I can not accept that the code I approved has obvious bugs.

Then we did find bugs, I don’t know why it should be complained. I would say that’s it’s lucky that I did a full review, otherwise the buggy code would make the code base unhealthy.

As discussed in discord/matrix, I can not understand the necessary of this PR

  1. I know your purpose is about federation, but I can not see how it’s done. The question is, if you have (repo, federation-sourcrce-1, local) and (repo, federation-sourcrce-2, local) records, the federation-sourcrce-1 is a UUID or an integer. how do you know which federation source do they come? Without a real usage case, I can not understand the design. If any maintainer understands the design and prefer to use it, please help to explain, thanks.
  2. About the purpose make migrations idempotent , it can be done by current existing issue.foreign_id , we do not need the extra table.

I made some changes on the PR because:

  1. The PR was set to Allow Edits from Maintainers by author.
  2. The models are moving into models/xxx separate packages, we shouldn’t put all the errors in the same file with ASCII banner. if you didn’t put the code into correct place, other maintainers would spend more time on it
  3. The usage of LocalID is quite misleading and buggy.

I am a developer, the only thing I care is how to make things right and make code clear and correct.

1 Like

I’m a coder too, but also a human being. And when I find myself distressed because of the interactions during a review, this is a problem. I am sorry that you think differently but I want to emphasize again that I have the utmost respect for your technical contributions, to this PR in particular and to Gitea in general. I also enjoy our discussions in the chat room, like the one about errors and traces.

Can you please, in the future, leave the review of the PRs I author to other Gitea maintainers?

1 Like

a bug is a bug and it does not care about your feelings.
It is one thing to expect a courteous and welcoming community, it is totally another to expect others to just accept your work because “it stresses you out” if it is questioned.

For the record, the follwup discussion in the develop chatroom.

@wxiaoguang you made it clear during our discussion in the chatroom that you will keep reviewing my PRs, regardless of my request. You do not review all pull requests, a number of them are merged without your review. You could help me contribute in a friendly environment by focusing on the PRs that I do not author and trust the other Gitea maintainers with mine.

You made it clear that you do not care about my feelings and I accept that. But if you keep causing me stress knowingly and for no reason, it will become harassment.

I will suspend my participation in the Gitea project until there is room for me to contribute in a safe environment.

  1. If you were questioning that why I didn’t review all PRs, please take a look at how many PRs I reviewed recently. I contribute in my spare time, I do not (and I do not have to) review all PRs. Pull requests · go-gitea/gitea · GitHub

  1. We were discussing about the bugs and the design, I never kept causing you stress, and I always explain reasons.

@dachary Hey. Take it easy, buddy. I think the purpose of all the approvals or change requests are making the code better quality. I also added some reviews in that PR after checking whether I should merge this PR. (Generally, when there are two approvals of a PR, a merger should review it again before merging.) When I found something, to avoid other mergers merge it accidentally, I will always add a block mark there. I think the block is not related with trust or not. PR authors can always post their viewpoints why they think the block is not right.

I also have many PRs(I’m very satisfy) are blocked because of different viewpoints. I trust the reviewers and blockers, I can still keep my passion for Gitea even they are not accepted.

To this PR, I think it’s very important and thank you for the contributions. @dachary
But as a step of a big change, the proposal and the design is very important. We should do it carefully. To introduce generics, Golang used 10 years. Most time are taken in discuss with community. To make this PR better, we also need more different voices. Even if another contributor has a reasonable viewpoint, I think we still need to resolve that.

At the end, thank you @dachary for your contributions. But open source almost means your code will be challenged by many people. I wish you can take it easy.
And please turn back to the PR itself and we can resolve those views together.

3 Likes

@lunny thank you for the kind words. And thank you for your continuous work on Gitea. I’m fine with having my PRs challenged for as long as it takes and rest assured that I will diligently address all review comments.

However, as I explained in this topic, I’m not OK with being harassed for no reason and subject to aggressive behavior. This is stressful and does not create a good environment for me to contribute.

I trust the suggestion to remedy to the situation will be followed and I will resume my contributions.

1 Like

@wxiaoguang I have addressed your review comment with this commit.

The adversarial tone of your review is causing me stress and makes my contributions unnecessarily difficult. @zeripath offered to act as an intermediary, which I’m grateful for as it would not expose me to the stress of our interactions. It would be inconvenient if that was repeated very often but if that’s only once every three months I trust it won’t be too much of a burden.

Thanks in advance for your understanding.

cc: @techknowlogick @lunny

Hi @dachary, take it easy. I think maybe he is not adversarial tone but because of frank character and Chinglish? I wish you reach a compromise.

@dachary @wxiaoguang

The compromise originally proposed by @zeripath is fine. It provides me with a stress free environment and allows @wxiaoguang to convey their review on my contributions.