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

Updated pull request template #32516

Merged
merged 2 commits into from
May 9, 2022

Conversation

DamianX
Copy link
Contributor

@DamianX DamianX commented May 3, 2022

What this does

Changed the GitHub pull request template to:

  • Remove outdated information
  • Reword and improve the explanations in the comments
  • Add a section in which the contributor is expected to describe what their PR does
  • Add a section in which the contributor is expected to explain why those changes are good in their opinion

Why it's good

I think explaining where one is coming from when proposing changes to the game is always beneficial and generates healthy discussion, as opposed to presenting a raw statement of what's been changed and leaving the readers to figure out the author's intention for themselves.

These are examples of PRs with no explanation: #32515, #32401,

@Kurfursten
Copy link
Collaborator

Kurfursten commented May 3, 2022

I think you have made good strides toward reducing our bloated template, however, I think having the full list of bot tag keys and changelog keys in the template is obnoxious. These would BOTH be better suited to a wiki page.

To that end I made one, so you can take out all the rest and just link it. https://ss13.moe/wiki/index.php/Guide_to_Writing_a_Pull_Request

@Eneocho
Copy link
Collaborator

Eneocho commented May 4, 2022

Could you add punctuation to the new lines?
Why It's Good: and What This Does:

@DamianX
Copy link
Contributor Author

DamianX commented May 9, 2022

I've been experimenting with a GitHub actions thingy that would let people self-label PRs (and issues!) without using MoMMI.
It doesn't support PRs until github/issue-labeler#41 gets merged, though. So for now, I'll just point to your wiki article.

However, I think the changelog entries should stay.
There is an argument to be made for the removal of some of the entries, such as soundadd, sounddel, tg, either wip or experiment, and spellcheck (since you wouldn't want a changelog for typos anyway).

Could you add punctuation to the new lines? Why It's Good: and What This Does:

I don't think that's how headings work.

@DamianX
Copy link
Contributor Author

DamianX commented May 9, 2022

How's this?

@Kurfursten
Copy link
Collaborator

Great

@Kurfursten Kurfursten merged commit 35152c3 into vgstation-coders:Bleeding-Edge May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants