godot-proposals icon indicating copy to clipboard operation
godot-proposals copied to clipboard

Create a tracker issue for typos and paper cuts

Open RedMser opened this issue 1 year ago • 6 comments

Describe the project you are working on

Godot Engine GitHub Issues

Describe the problem or limitation you are having in your project

Whenever a user finds a typo or a UI paper cut in the Godot code, they are told not to fix it as a standalone PR, since it would clutter the PRs. So instead they're told to bunch together a lot of issues, then fix them in one go - or fix them as part of a larger refactoring or feature.

However, as it's unlikely that many users are willing to do more work than intended, these issues just die out and the typo or UI paper cut persists.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Create a tracker issue in the Godot repository that keeps track of tiny issues which can easily be fixed and are not opinionated changes. This includes:

  • A misspelling in the UI or class reference
  • A misspelling in the code (comment, variable name, class name, etc. as long as it is not a breaking change)
  • Adding a property hint to a property
  • Giving names to unnamed method arguments

No functional changes are tracked here, so no refactoring, reordering code.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Users can request entries by posting comments, where they will be collected in the tracker issue's description. A code location should be provided as well (not pointing to latest master, but rather to the currently latest commit hash).

Fixes from the list can be applied in following cases:

  • A user is working in an affected area of the code, and can just fix the offending paper cut while they're at it.
  • Once there are enough paper cuts collected, they can all be applied as one large batch commit. This should ideally be done at a certain time during the release cycle, open to suggestions on details here.

If this enhancement will not be used often, can it be worked around with a few lines of script?

N/A

Is there a reason why this should be core and not an add-on in the asset library?

N/A

RedMser avatar May 01 '24 20:05 RedMser

See:

  • https://github.com/godotengine/godot/pull/46975#issuecomment-799639145
  • https://github.com/godotengine/godot/pull/76206#issuecomment-1513129666

dalexeev avatar May 02 '24 03:05 dalexeev

I think this is a great idea. I often come across typoed, weird or even misleadingly named variables, blocks of code which purpose needs a cup to coffee to understand what's its purpose (lacking helpful comments) etc. but don't bother doing anything with them. If there was a tracker for these small things maybe I would start collecting and submitting them.

Maybe there could be a bot which would analyze incoming pull requests and automatically notify the author that there's small issues around the edited code if they are catalogued in the tracker.

aXu-AP avatar May 02 '24 18:05 aXu-AP

We shouldn't change code unrelated to PRs unless it's strictly non-functional, i.e. style

Functional changes should always be in a dedicated PR to make it manageable to track issues from it

AThousandShips avatar May 02 '24 18:05 AThousandShips

Maybe there could be a bot which would analyze incoming pull requests and automatically notify the author that there's small issues around the edited code if they are catalogued in the tracker.

I wonder if such a bot already exists. Creating one from scratch seems like more effort than it's worth I feel. Also Godot doesn't use any GitHub bots yet, so this would need a consensus by the maintainers first.


Functional changes should always be in a dedicated PR to make it manageable to track issues from it

Do you consider any of the three things the issue should track to be functional changes? I could clarify the proposal wording.

I wouldn't accept renaming public API as a change, but e.g. local variables or private fields would be fine. General "rename things for fun/consistency" should be forbidden too - it should explicitly be for things that are named wrongly.

RedMser avatar May 02 '24 18:05 RedMser

Even something as simple as reordering a loop would IMO be cause to make a dedicated PR, as it can cause unforseen issues

AThousandShips avatar May 02 '24 18:05 AThousandShips

@AThousandShips I updated the proposal to make it clear that changes should be limited to style changes. I totally agree that refactors should not count here, so I clarified "paper cut -> UI paper cut" as well. Let me know if the wording can be improved further.

RedMser avatar May 02 '24 19:05 RedMser

Since it generally seems like there is interest in this, I've created https://github.com/godotengine/godot/issues/91521 as the tracker issue.

Feel free to contribute typos/papercuts you're aware of, either by editing the issue directly (for maintainers) or by commenting. Any changes on the wording are also welcome!

If the maintainers deem this unnecessary or want to tackle the problem differently, feel free to close the tracker issue.

RedMser avatar May 03 '24 16:05 RedMser