Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CONTRIBUTING.md #24492

Merged
merged 16 commits into from
May 23, 2023
Merged

Update CONTRIBUTING.md #24492

merged 16 commits into from
May 23, 2023

Conversation

jolheiser
Copy link
Member

@jolheiser jolheiser commented May 2, 2023

Previously, the CONTRIBUTING was severely outdated in certain aspects such as workflows.
These sections have been brought up to date.
Furthermore, the CONTRIBUTING now mentions the TOC, how it is structured, elected, and it's duties.


Note, this PR is made on behalf of a collection of maintainers who collaborated on this prior to the creation of a PR. Please feel free to review!

@jolheiser jolheiser added the type/docs This PR mainly updates/creates documentation label May 2, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 2, 2023
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 2, 2023
@delvh
Copy link
Member

delvh commented May 2, 2023

Hmm… I think I have to refrain from approving, given that I wrote probably 50-70% of this PR.

CONTRIBUTING.md Outdated Show resolved Hide resolved
## Design guideline
Gitea uses it's own tool, the <https://github.com/GiteaBot/gitea-backporter> to automate parts of the review process. \
This tool does the things listed below automatically:
- create a backport PR if needed once the initial PR was merged
Copy link
Member Author

@jolheiser jolheiser May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It attempts to do this, but is not always successful, e.g. when conflicts occur
Do we want to mention that here?

EDIT: I see it explains it further down, maybe we could link to that part here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe we could improve the whole documentation detail in the docs section Contributing but not make this file bloat. About the robot, they are changed frequently recently.

CONTRIBUTING.md Outdated
Comment on lines 286 to 288
If a PR has been ignored for more than 7 days with no comments or reviews, and the author or any maintainer believes it will not survive a long wait (such as a refactoring PR), they can send "one last call" to the TOC by mentioning them in a comment.

If you add a new feature or change an existing aspect of Gitea, the documentation for that feature must be created or updated.
After another 7 days, if there is still zero approval, this is considered a polite refusal, and the PR will be closed to avoid wasting further time. Therefore, the "final call" has a cost, and should be used cautiously.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I like the wording here.
Saying the final call has a cost whereby potentially a PR will simply be closed doesn't sound entirely friendly.

I think in most cases a response of some form will be given, rather than a quiet closing. If that's not the case, it should be the case imo

Copy link
Member

@wolfogre wolfogre May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think once a final call has been sent, the maintainers should comment/review ASAP to avoid a quiet closing.

So whether it is friendly depends on how the maintainers respond to the final call. And if someone always sends final calls for their unimportant PRs, I think "a quiet closing" is the most friendly way. Just my opinion, and I don't think it's really going to happen.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@wxiaoguang
Copy link
Contributor

Should it be splitted into some small files? I am not sure contributors have the patience and courage to open and read such a large document ....

@jolheiser
Copy link
Member Author

Should it be splitted into some small files? I am not sure contributors have the patience and courage to open and read such a large document ....

Possibly fair, but splitting it up just means multiple documents to read instead.
I'm not against splitting it, though.
Anyone else +1 or -1 on that?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented May 3, 2023

Possibly fair, but splitting it up just means multiple documents to read instead.

Split by topic? Most contributors do not need to know "backport" or "frontport" or "releasing"

Maybe this "contributing.md" could be an entry / index for sub topic, there are already many link in it, eg: guidelines, etc.

@lunny
Copy link
Member

lunny commented May 3, 2023

Possibly fair, but splitting it up just means multiple documents to read instead.

Split by topic? Most contributors do not need to know "backport" or "frontport" or "releasing"

Maybe this "contributing.md" could be an entry / index for sub topic, there are already many link in it, eg: guidelines, etc.

Some documentations could be as part of documentation and there are references from the CONTRIBUTING.md . But for the file, the stanard of many tools are only 1 file.

@wxiaoguang
Copy link
Contributor

the stanard of many tools are only 1 file.

I do not think so.

Although many tools have "single-page" document, these documents are maintained by separate pages, the single-page document is just automatically generated. If you visit their web site, in most cases, there is also a link to separate pages for the documents.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 15, 2023
CONTRIBUTING.md Outdated Show resolved Hide resolved
@lunny lunny marked this pull request as ready for review May 22, 2023 05:58
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 22, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 22, 2023
@delvh delvh removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 22, 2023
@delvh
Copy link
Member

delvh commented May 22, 2023

I'm temporarily removing it from the merge queue for about half a day so that we can gather all maintainers that worked on this update in the commit message.

@delvh
Copy link
Member

delvh commented May 23, 2023

Okay, apparently the only other people who worked on this are @techknowlogick and me.

@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 23, 2023
@delvh delvh changed the title CONTRIBUTING.md updates Update CONTRIBUTING.md May 23, 2023
@delvh delvh merged commit 116066e into go-gitea:main May 23, 2023
@GiteaBot GiteaBot added this to the 1.20.0 milestone May 23, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 23, 2023
silverwind added a commit to silverwind/gitea that referenced this pull request May 23, 2023
* main:
  Remove publish docs CI workflow (go-gitea#24889)
  Fix double border and adjust width for user profile page (go-gitea#24870)
  Support changing git config through `app.ini`, use `diff.algorithm=histogram` by default (go-gitea#24860)
  Fix flakey test in logger test (go-gitea#24883)
  Run stylelint on .vue files (go-gitea#24865)
  Update `CONTRIBUTING.md` (go-gitea#24492)
  Do not call nil handler for a dummy queue (go-gitea#24880)
  Remove unnecessary usage prefix from doc titles (go-gitea#24869)
  Add API for Label templates (go-gitea#24602)
  Fix install page context, make the install page tests really test (go-gitea#24858)
  Add validations.required check to dropdown field (go-gitea#24849)
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 24, 2023
* upstream/main:
  Fix `@font-face` overrides (go-gitea#24855)
  Fix logger refactoring regression: manager logging add (go-gitea#24847)
  Remove publish docs CI workflow (go-gitea#24889)
  Fix double border and adjust width for user profile page (go-gitea#24870)
  Support changing git config through `app.ini`, use `diff.algorithm=histogram` by default (go-gitea#24860)
  Fix flakey test in logger test (go-gitea#24883)
  Run stylelint on .vue files (go-gitea#24865)
  Update `CONTRIBUTING.md` (go-gitea#24492)
  Do not call nil handler for a dummy queue (go-gitea#24880)
  Remove unnecessary usage prefix from doc titles (go-gitea#24869)
  Add API for Label templates (go-gitea#24602)

# Conflicts:
#	.stylelintrc.yaml
#	web_src/css/font_i18n.css
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants