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

PR template: remove don't delete HTML comments; add Summary examples; grammar #72379

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

Brambor
Copy link
Contributor

@Brambor Brambor commented Mar 13, 2024

Summary

None

Purpose of change

I thought the tests on a PR would fail if the HTML comments were deleted.

from Discord @GuardianDll

it says this so first time contributors won't remove it without reading

I don't believe this will start happening now.

Describe the solution

Remove the sentence that led me to believe that.
Also, fix some grammar reported by Gramarly, so that the template isn't red for me.

Describe alternatives you've considered

N/A

Testing

I see that I am writing this PR with the new template!

I am assuming the PR tester will not throw any errors as I have removed HTML comments in this PR.

Additional context

N/A

@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Mar 13, 2024
@Brambor Brambor changed the title PR template: remove don't delete the comments; grammar PR template: remove don't delete the comments; grammar Mar 13, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 14, 2024
@Maleclypse
Copy link
Member

"I don't believe this will start happening now." That would be incredible since it already happens now and I have to fix new contributors PR Summaries on the regular.

@Brambor
Copy link
Contributor Author

Brambor commented Mar 14, 2024

"I don't believe this will start happening now." That would be incredible since it already happens now and I have to fix new contributors PR Summaries on the regular.

I mean it will not affect the situation. It will not happen more nor less.

Do you think the sentence helps? I don't feel strongly about removing it. I would like the next contributors to not be as confused as I was. Managing the PR is easier when you remove the HTML comments.

If you prefer, I will leave it in and reduce this to grammar fixes. I mean, you're the one who deals with it daily. And I thank you for that!

@Brambor
Copy link
Contributor Author

Brambor commented Mar 14, 2024

I decided to add some summary examples too. They are taken from the existing changelog. A real example might get across better than a generic template.

@Brambor Brambor changed the title PR template: remove don't delete the comments; grammar PR template: remove don't delete HTML comments; add Summary examples; grammar Mar 14, 2024
@Maleclypse
Copy link
Member

I decided to add some summary examples too. They are taken from the existing changelog. A real example might get across better than a generic template.

I like those examples. We’ll try it this way and see if it improves. Let me know when ready to merge.

@Brambor
Copy link
Contributor Author

Brambor commented Mar 14, 2024

Let me know when ready to merge.

It's ready :).

@Maleclypse Maleclypse merged commit 4ed097b into CleverRaven:master Mar 15, 2024
15 of 21 checks passed
@Brambor Brambor deleted the pr-template branch March 16, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants