flixel icon indicating copy to clipboard operation
flixel copied to clipboard

DalekCraft2 doc changes

Open Geokureli opened this issue 2 years ago • 5 comments

@DalekCraft2

Simply put, verifying a 14000 line change for doc comments is unmanageable, but lets see if we can work something out.

in the future I only want format/doc changes in the following ways (this is typically how i do it):

  1. If you make a change to some code to fix a bug or add a feature, you can change the formatting, comments, documentation of that method, if the file is small and isolated, you might want to make similar changes to the other methods.
  2. You may make project-wide changes of one singular type. For instance, if you think it's helpful for all digits in doc comments to have backticks surrounding them you can make a PR with that change, solely, and I'm far more likely to review and accept it. if 2 change types are closely related or relevant, they may be both changed in the same PR. but the bigger a PR is the more likely it will conflict with other ongoing feature development and the more likely I wont have the time or effort to review it.

Regarding #2761, if you can list every type of change that you made there in a reply to this thread, I'll let you know which individual changes I'm interested in. Just know that I'm only interested in wording or grammatical changes if the original doc is misleading or sounds like it was written by someone who hasn't learned english, yet. For instance, I'm not interested in adding oxford commas unless it removes some kind of ambiguity (ex: "We invited the strippers, Hitler and Stalin." are they the strippers or were they invited, as well?)

For instance if you wanted to make a PR that changes every "Flag for whether ..." to "Whether ..." that would be fine, but I still argue that there are far better uses of your time. What flixel could really use right now is better unit tests, or more demos and snippets. And I still strongly recommend heavily focusing on personal projects that use HaxeFlixel as a means to find problems and feature gaps, rather than making a million little changes.

A lot of people would love to have someone with your enthusiasm, dedication and attention to detail on their side, especially in an often neglected area like documentation, I'd like to see your efforts continued, but I think you need to be reined in, I think your expectations need to be a bit more realistic, and - not to put too fine a point on it - I think your focus could be directed towards more important issues.

Geokureli avatar Mar 30 '23 16:03 Geokureli

I think your focus could be directed towards more important issues.

Fair, and I agree. My focus needs to be directed in general; I tend to get very carried away with stuff like this, which will probably be very evident in this list.

Note: Some of these I only started doing partway through, so files what I edited earlier on may not have those changes.

General grammar and text stuff

  • Corrected typos in doc comments and one in README.md
  • Used Oxford comma wherever applicable
  • Removed many instances of "Just" because they seemed unnecessary
  • Replaced some commas with periods/semicolons when they were part of run-on sentences
  • Replaced some dashes with periods and some with semicolons
  • Added commas in some doc comments to separate the parts of the sentences better
  • Added dashes after every single mention of an x- or y-coordinate. Sometimes replaced "x-/y-coordinates" with "position" for consolidation.
  • Made every instance of "x" and "y" lowercase
  • Removed apostrophes from some instances of "it's" and added some to some instances of "its" when they were not grammatically correct in context (did this with other things, too, like "tilemap's")
  • That one "vs." change...
  • Used "whether" instead of "if" for boolean field/arguments, and got rid of "or not" in some boolean doc comments because they are unnecessary
  • Put "e.g." and "i.e." in parentheses when they are in the middle of a sentence and added commas right after them (after the "e.g.", not after the parentheses)
  • Added periods after "etc."
  • Used hyphens between some terms like "pixel-perfect"
  • Added periods after everything and capitalized the start of everything
  • Capitalized "ID" and "FPS"
  • Changed some instances of "tall" to "high" when referring to height
  • Removed some instances of doc comments saying "you" just because I tend to write doc comments without referring to the user at all, but did not finish removing all of them
  • Used present simple tense as often as possible to describe fields and such
  • Replaced some instances of "FPS" with "framerate" when it was not being used as a unit
  • Changed some regional spellings because the majority of the rest of the code used a different spelling (i.e., changed "behaviour" and "colour" to "behavior" and "color" because the latter two were more common)
  • Capitalized things like "Flixel", "Flash", "CPP", "OpenFL", "Lime", "SteamWrap", etc.
  • (Valuable) Reworded a lot of the doc comments in the flixel.debug.interaction package because they tended to use the wrong words often

Comments and documentation

  • Surrounded all of the following with backticks:
    • true and false
    • Numbers, when they refer to code
    • Names of Classes/Enums/Typedefs/etc.
    • Field names
    • Parameter names
    • null
    • Metadata
    • Comparators like >= and ==. These tended to be bundled in the same backtick "group" as what they were comparing (e.g., >= 0).
    • File names
    • Coordinates, but I only did a few because I only decided to do that late in the edits
  • Added () to the end of every mention of a function in a doc comment
  • Used # to separate classes from functions/fields when the field is an instance field
  • Rephrased the doc comments of some fields which were phrased as if the field was a function
  • Removed doc comments on overridden functions which were the same as the doc comment on their parent function (though, for this to be useful, I would need to add a bunch of @:inheritDoc tags to everything; otherwise, the IDE will not show the docs at all when hovering over the overridden function)
  • (I consider this to be a valuable change) Added a description for the elapsed parameter in update()
  • In the cases of things referring to FlxObject's collision constants, changed them to refer to FlxDirectionFlags
  • (Semi-valuable?) Similarly to the above change, changed other things what referred to deprecated things, such as FlxObject#path's comment saying to refer to flixel.util.FlxPath when that has been deprecated for flixel.path.FlxPath
  • (Semi-valuable?) Changed the deprecation messages for FlxObject's collision constants from saying stuff like "Use LEFT or FlxDirectionFlags.LEFT instead" to saying "Use FlxDirectionFlags.LEFT instead" because the extra "LEFT" was just kind of confusing
  • (Valuable) Corrected misspelt parameters in doc comments, like what I did in the flixel-addons PR
  • (Valuable) Made the doc comment for FlxObject#reset() more accurate, and changed some other doc comments what referred to it and FlxBasic#revive() to be more accurate as well
  • (Valuable) Changed FlxState#persistentDraw's comment from saying "whether it is updated" to saying "whether it is drawn"
  • Changed some "Todo" comments to be "TODO" in case you are using the same extension as I am which only detects TODO comments when they are fully capitalized; also moved one of them from a doc comment to a normal comment so it would be detected
  • Changed some doc comments with links to use Markdown's links "like this" instead of "like this (https://example.website)"
  • Sorted some doc comment tags based on Java's rules for them
  • (Valuable) Fixed doc comments what were copied from elsewhere so they refer to the class in which they currently are instead of the class from which they are copied (e.g., some doc comments from flixel.util.helpers referenced FlxEmitter and FlxParticle). There were quite a few instances of this.
  • (Valuable) Removed mentions of FlxPlugin (which does not exist) and replaced them with FlxBasic, and removed mentions of the flixel.plugins package (which also does not exist)
  • Removed @return tags from the doc comments of constructors
  • Changed FlxBar's doc comments to say "bar" instead of "health bar" (except for the one instance where it is giving an example of using FlxBar as a health bar)
  • Changed some links from http to https if they were naturally https links
  • Removed the link http://www.flashandmath.com/advanced/p10triangles/index.html from FlxStrip because it does not exist; if it turns out that its domain simply changed or something, I'll add it back with an updated link
  • Removed a dead link http://spritesheetpacker.codeplex.com/ from FlxAtlasFrames#fromSpriteSheetPacker()'s comment

Actual code

  • Removed some unused imports
  • (Valuable) Fixed a deprecated comment which was typed as @deprecated instead of @:deprecated
  • (Valuable) Changed the warning message for FlxG.set_updateFramerate() because it seemed to be outdated, especially compared to the FlxG.set_drawFramerate() warning
  • (Valuable) Added a throws line to FlxSpriteGroup#loadGraphic() because some other overridden functions in that class threw exceptions because "FlxSpriteGroup did not support that functionality" and loadGraphic() was the only one what did not throw one. In other words, it was the only... exception. I'll be here all night, folks.
  • (Not really a change) Almost made FlxAnimation#delay not have a set() function because frameDuration does not have one, but decided not to because that could possibly break something in an existing project
  • Added a single space in FlxStringUtil#formatBytes() to separate the number from the unit
  • Removed a single stray closing parenthesis from an exception message in FlxAtlas#addNodeWithSpacings()
  • (Valuable) Changed FlxDebugger#openHomepage() to use haxeflixel.com instead of www.haxeflixel.com because the latter does not exist
  • In the cases of classes which were entirely inside compilation conditionals, moved their doc comments inside the conditionals (same with imports)

Getting carried away

  • Changed every Flash import to an OpenFL import
  • Added semicolons to the end of every typedef
  • Changed the name of one of the unit testing functions from assertdegreesTo() to assertDegreesTo()

File formatting

  • Got rid of the many extra spaces/tabs in the doc comments, mostly in the @param and @return tags
  • Ran the formatter everywhere, but tried to keep some intentional formatting choices like spacing apart entries in arrays so they form a matrix

Miscellaneous

  • Made all of the Lime Project.xml files use version 1.0.4 of the schema instead of 1.0.2

DalekCraft2 avatar Mar 30 '23 18:03 DalekCraft2

Things I'm not interested in

Many of these are too subjective, regional, or preferential. We have people all over the word using and writing haxe documentation I see no benefit to having this amount of rigidity in the style and prose of HF's docs. I also don't want to have too many rules adding friction to new contributors

  • Used Oxford comma wherever applicable
  • Added dashes after every single mention of an x- or y-coordinate. Sometimes replaced "x-/y-coordinates" with "position" for consolidation.
  • Replaced some dashes with periods and some with semicolons
  • Added commas in some doc comments to separate the parts of the sentences better
  • That one "vs." change... (G: less readible imo)
  • Added dashes after every single mention of an x- or y-coordinate. Sometimes replaced "x-/y-coordinates" with "position" for consolidation.
  • Used hyphens between some terms like "pixel-perfect"
  • Added periods after "etc."
  • Capitalized "ID" and "FPS"
  • Changed some instances of "tall" to "high" when referring to height
  • Replaced some instances of "FPS" with "framerate" when it was not being used as a unit
  • Added () to the end of every mention of a function in a doc comment

Things you should change only if you're already changing the wording of comments

These are things I agree with (often to a small degree), but don't thing warrant our time unless you're already there changing shit.

  • Removed many instances of "Just" because they seemed unnecessary
  • Replaced some commas with periods/semicolons when they were part of run-on sentences
  • Removed apostrophes from some instances of "it's" and added some to some instances of "its" when they were not grammatically correct in context (did this with other things, too, like "tilemap's")
  • Made every instance of "x" and "y" lowercase
  • Added periods after everything and capitalized the start of everything (G: please don't go nuts with this)
  • Removed some instances of doc comments saying "you" just because I tend to write doc comments without referring to the user at all, but did not finish removing all of them
  • Used present simple tense as often as possible to describe fields and such
  • Changed some regional spellings because the majority of the rest of the code used a different spelling (G: "colour" should be "color", because of FlxColor, but nothing else matters)

Things I need examples of

Some things weren't clear and i didn't want to search the PR for an example. please provide a link to the line of one of these changes

  • Put "e.g." and "i.e." in parentheses when they are in the middle of a sentence and added commas right after them (after the "e.g.", not after the parentheses)
  • Used # to separate classes from functions/fields when the field is an instance field
  • Rephrased the doc comments of some fields which were phrased as if the field was a function
  • Sorted some doc comment tags based on Java's rules for them
  • (Not really a change) Almost made FlxAnimation#delay not have a set() function because frameDuration does not have one, but decided not to because that could possibly break something in an existing project
  • Added a single space in FlxStringUtil#formatBytes() to separate the number from the unit
  • Removed a single stray closing parenthesis from an exception message in FlxAtlas#addNodeWithSpacings()

things you can change in a dedicated PR

Each item should be it's own let me know if you think some should be combined. Typically if there's a one-off case, I make a note of it and make a separate PR, or I just stash my current changes, create a new branch off dev, make the change, publish and move back to my previous branch.

  • (Valuable) Reworded a lot of the doc comments in the flixel.debug.interaction package because they tended to use the wrong words often
  • Surrounded all of the following with backticks: (G: unless crossed out)
    • true and false
    • Numbers, when they refer to code
    • Names of Classes/Enums/Typedefs/etc.
    • Field names
    • Parameter names
    • null
    • Metadata
    • Comparators like >= and ==. These tended to be bundled in the same backtick "group" as what they were comparing (e.g., >= 0).
    • ~~File names~~ (G: leave alone for now)
    • ~~Coordinates, but I only did a few because I only decided to do that late in the edits~~ (G: example please)
  • deprecated references
    • In the cases of things referring to FlxObject's collision constants, changed them to refer to FlxDirectionFlags
    • (Semi-valuable?) Similarly to the above change, changed other things what referred to deprecated things, such as FlxObject#path's comment saying to refer to flixel.util.FlxPath when that has been deprecated for flixel.path.FlxPath
    • (Semi-valuable?) Changed the deprecation messages for FlxObject's collision constants from saying stuff like "Use LEFT or FlxDirectionFlags.LEFT instead" to saying "Use FlxDirectionFlags.LEFT instead" because the extra "LEFT" was just kind of confusing
  • Changed some doc comments with links to use Markdown's links "[like this](https://example.website)" instead of "like this (https://example.website)"
  • (Valuable) Removed mentions of FlxPlugin (which does not exist) and replaced them with FlxBasic, and removed mentions of the flixel.plugins package (which also does not exist)
  • Removed @return tags from the doc comments of constructors
  • Changed FlxBar's doc comments to say "bar" instead of "health bar" (except for the one instance where it is giving an example of using FlxBar as a health bar)
  • Added semicolons to the end of every typedef
  • Removed some unused imports
  • (Valuable) Fixed a deprecated comment which was typed as @deprecated instead of @:deprecated (G: "deprecated" was a comment or code? this was listed under "Actual Code")
  • (Valuable) Changed the warning message for FlxG.set_updateFramerate() because it seemed to be outdated, especially compared to the FlxG.set_drawFramerate() warning
  • (Valuable) Changed FlxDebugger#openHomepage() to use haxeflixel.com instead of www.haxeflixel.com because the latter does not exist
  • Changed the name of one of the unit testing functions from assertdegreesTo() to assertDegreesTo()
  • Made all of the Lime Project.xml files use version 1.0.4 of the schema instead of 1.0.2

Things you can change in a series of small PRs

Try to keep PRs around 500 line changes (i said your last pr was 14000 changes but I really count it as 7000). If these turn out to be larger than 500 as an idividual task, you don't need to split it up into many PRs. I'd still prefer all of these to be done using individual commits so that the git blame shows a relevant commit message on every line rather than just "various doc changes".

  • Used "whether" instead of "if" for boolean field/arguments, and got rid of "or not" in some boolean doc comments because they are unnecessary
  • Capitalized things like "Flixel", "Flash", "CPP", "OpenFL", "Lime", "SteamWrap", etc.
  • (I consider this to be a valuable change) Added a description for the elapsed parameter in update()
  • (Valuable) Corrected misspelt parameters in doc comments, like what I did in the flixel-addons PR
  • (Valuable) Made the doc comment for FlxObject#reset() more accurate, and changed some other doc comments what referred to it and FlxBasic#revive() to be more accurate as well
  • (Valuable) Changed FlxState#persistentDraw's comment from saying "whether it is updated" to saying "whether it is drawn"
  • Changed some "Todo" comments to be "TODO" in case you are using the same extension as I am which only detects TODO comments when they are fully capitalized; also moved one of them from a doc comment to a normal comment so it would be detected
  • (Valuable) Fixed doc comments what were copied from elsewhere so they refer to the class in which they currently are instead of the class from which they are copied (e.g., some doc comments from flixel.util.helpers referenced FlxEmitter and FlxParticle). There were quite a few instances of this.
  • Changed some links from http to https if they were naturally https links
  • Removed the link http://www.flashandmath.com/advanced/p10triangles/index.html from FlxStrip because it does not exist; if it turns out that its domain simply changed or something, I'll add it back with an updated link
  • Removed a dead link http://spritesheetpacker.codeplex.com/ from FlxAtlasFrames#fromSpriteSheetPacker()'s comment (G: is there a new link?)

Bigger discussions for later

  • Let's make a Style Guide
  • Removed doc comments on overridden functions which were the same as the doc comment on their parent function (though, for this to be useful, I would need to add a bunch of @:inheritDoc tags to everything; otherwise, the IDE will not show the docs at all when hovering over the overridden function)
  • Changed every Flash import to an OpenFL import (G: guaranteed to cause conflicts with other branches)
  • (Valuable) Added a throws line to FlxSpriteGroup#loadGraphic() because some other overridden functions in that class threw exceptions because "FlxSpriteGroup did not support that functionality" and loadGraphic() was the only one what did not throw one. In other words, it was the only... exception. I'll be here all night, folks.
  • In the cases of classes which were entirely inside compilation conditionals, moved their doc comments inside the conditionals (same with imports)
  • Got rid of the many extra spaces/tabs in the doc comments, mostly in the @param and @return tags

Things I'm pretty sure I told you to never do

  • Ran the formatter everywhere, but tried to keep some intentional formatting choices like spacing apart entries in arrays so they form a matrix

Geokureli avatar Mar 30 '23 22:03 Geokureli

About things you don't want

Yeah, I agree for all of them. They are not necessary.

About things for which you need examples

  • "e.g. and i.e.":

    • Doc comment of FlxSpriteGroup#getFirstAvailable(): Changed "This is handy for recycling in general, e.g. respawning enemies" to "This is handy for recycling in general (e.g., respawning enemies)". Surrounded the examples in parentheses, and added a comma right after the "e.g." because, uh, Google said to add a comma. image
  • "separate instance fields with '#'":

  • "fields with doc comments what make them sound like functions":

    • Doc comment of FlxAnimationController#frameName: Changed from "Tell the sprite to change to a frame with specific name" to "The name of the current frame. Can be changed manually to set the current frame" because the former sounded like it would be more fitting if frameName was a function, which it is not image
  • "doc comment tag sorting":

  • FlxAnimation#delay: delay is deprecated. Its replacement, frameDuration, has a default getter and a null setter. delay has a get getter and a set setter. delay's setter allows for changing the value of frameDuration from outside the class without using @:privateAccess. I almost changed delay to have a never setter, but decided against it. This technically is not a change at all; it is just something that was almost a change. image

  • FlxStringUtil.formatBytes(): I added that little " ". image

  • FlxAtlas#addNodeWithSpacings(). (I can see why you may have been confused; I was referring to the function the same way the exceptions did, but the exceptions did not use the current name of the function, so I ended up referring to a nonexistent one. My bad.) image

  • "Putting coordinates in backticks":

    • Doc comment of FixedScaleAdjustSizeScaleMode: I only started doing this change a bit after I saw a single instance of a coordinate which was already in backticks in a doc comment, so there are very few instances of it. I don't remember where that first instance was, though. image

About things what can go in dedicated PRs

When you say each list item should go in its own PR, do you also mean that the PRs for backticking null and backticking true/false should be separate PRs?

For the @:deprecated thing, I used the wrong word; I meant to say "metadata" instead of "comment". The metadata was typed as @deprecated instead of @:deprecated, so it was not picked up by the compiler or IDE as metadata. I'm not really sure why that would even compile without the colon, but it did. Here's where it was, by the way. image

About things what can go in a series of small PRs

Not really sure about whether there are new links for flashandmath.com or spritesheetpacker.codeplex.com. I do not know what either website looked like before, so I am unsure of for what kind of resource I am looking. There is apparently a Git mirror of the latter, but I am not sure whether that is the kind of resource what you want.

DalekCraft2 avatar Mar 31 '23 15:03 DalekCraft2

When you say each list item should go in its own PR, do you also mean that the PRs for backticking null and backticking true/false should be separate PRs?

all the sub bullet points can go in a single PR, similarly all the deprecated references sub points can be a single pr too

Not really sure about whether there are new links for...

we can talk nitty-gritty for each issue when the PR's are created, use your best judgement and I'll let you know if there's a better course of action. but for this, i checked The Wayback Machine and it seems like a small tutorial, it was redirected to a github repo by Nick Gravelyn who seems to have deleted his guthub and has fallen off the universe. Removing the link should be fine, maybe in the next major version we should consider removing fromSpriteSheetPacker

I'll move or label items from the "need examples" category soon

Geokureli avatar Mar 31 '23 15:03 Geokureli

Yes

  • Added a single space in FlxStringUtil#formatBytes() to separate the number from the unit
  • Removed a single stray closing parenthesis from an exception message in FlxAtlas#addNodeWithSpacings()
  • Putting coordinates in backticks

No

  • Put "e.g." and "i.e." in parentheses when they are in the middle of a sentence and added commas right after them
  • Used # to separate classes from functions/fields when the field is an instance field
  • Rephrased the doc comments of some fields which were phrased as if the field was a function
  • Sorted some doc comment tags based on Java's rules for them
  • (Not really a change) Almost made FlxAnimation#delay not have a set() function because frameDuration does not have one, but decided not to because that could possibly break something in an existing project

Geokureli avatar Mar 31 '23 17:03 Geokureli