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

Fix grammar in Marker2D.xml #76206

Closed
wants to merge 1 commit into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Apr 18, 2023

Changes "You can set cross' visual size..." to "You can set the cross' visual size..." on https://docs.godotengine.org/en/stable/classes/class_marker2d.html

@ghost ghost self-requested a review as a code owner April 18, 2023 12:37
@YuriSizov YuriSizov added this to the 4.1 milestone Apr 18, 2023
@YuriSizov
Copy link
Contributor

Thanks for making a PR! We usually prefer that PRs to the main repository do a bit more substantial changes. Do you know of any other missing articles or present grammar issues in this or other relevant docs? Could you take a look?

@ghost
Copy link
Author

ghost commented Apr 18, 2023

I'm constantly reading through the documentation, so I can add to this PR if/when I come across any other class-related changes.

@dalexeev
Copy link
Member

We usually prefer that PRs to the main repository do a bit more substantial changes.

Perhaps we should create some kind of tracker to collect single typos and periodically fix them in batches?

@YuriSizov
Copy link
Contributor

Perhaps we should create some kind of tracker to collect single typos and periodically fix them in batches?

Well, we are currently on a bug tracker platform, this can be reported as an issue as well :)

The problem is with attribution and making sure that main repo commits have significance. If someone wants to make a PR, but it doesn't pass the bar of significance, then what should we do? If the policy is to reject it, track it, and then make a batch, then the author of that batch will take all the credit. Which is not very encouraging for the original poster.

So it's better to ask first-time contributors bringing these PRs to look for more cases. Chances are, they can add more meat to their contribution and take their deserved credit. And if in the meantime someone else does a PR that incorporates their change, intentionally or not, then so be it.

@MewPurPur
Copy link
Contributor

MewPurPur commented Apr 18, 2023

Wait a sec, I get the grammatical error, but I don't get what the whole sentence is referring to. As far as I know, you can only edit this value from the inspector, right? Also this doesn't seem to be in Marker3D's docs. Maybe I'm missing something.

I guess you can make this pass the bar of significance by polishing up the whole documentation of these nodes. Just from a cursory look, for example, the brief description says this is a "Generic position hint", where "generic" doesn't add any information and is akin to 'banned' words like "simple" and "basic".

Oh, and if you do update the PR, make sure to amend your commit so you don't have to squash your commits later.

@YuriSizov
Copy link
Contributor

I guess you can make this pass the bar of significance by polishing up the whole documentation of these nodes.

Fixing only grammar here would be fine. We don't want our contributors to go down a rabbit hole like that when all they have right now is one missing article :)

akien-mga added a commit that referenced this pull request Apr 26, 2023
Also includes the grammar fix from #76206.

Co-authored-by: Peter Anderson <BWPanda@users.noreply.github.com>
@akien-mga
Copy link
Member

I've included this fix directly in e1075e9 while fixing various other typos, and credited you as co-author. Thanks for the contribution!

@akien-mga akien-mga closed this Apr 26, 2023
@ghost ghost deleted the patch-1 branch April 26, 2023 12:08
JeffVenancius pushed a commit to JeffVenancius/godot that referenced this pull request Apr 27, 2023
Also includes the grammar fix from godotengine#76206.

Co-authored-by: Peter Anderson <BWPanda@users.noreply.github.com>
hakro pushed a commit to hakro/godot that referenced this pull request May 21, 2023
Also includes the grammar fix from godotengine#76206.

Co-authored-by: Peter Anderson <BWPanda@users.noreply.github.com>
avilches pushed a commit to avilches/godot that referenced this pull request May 26, 2023
Also includes the grammar fix from godotengine#76206.

Co-authored-by: Peter Anderson <BWPanda@users.noreply.github.com>
TheSecondReal0 pushed a commit to TheSecondReal0/godot that referenced this pull request Jul 20, 2023
Also includes the grammar fix from godotengine#76206.

Co-authored-by: Peter Anderson <BWPanda@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants