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

Link to demos from within the class reference #42486

Merged
merged 1 commit into from
Oct 2, 2020

Conversation

aaronfranke
Copy link
Member

This was briefly discussed on IRC awhile back and reduz thinks it's a good idea. There is probably a lot more that can be linked, but this work is really monotonous and takes a long time, so I'm burned out for now and I'm submitting what I have so far.

I was wondering what the criteria should be for linking to a demo, since many classes have more than one demo, and we don't have (and can't realistically have) a demo for each individual feature. For this PR, I decided on a generic general rule of "if the demo helps users understand the feature, link it".

There is also some linking of non-demo things in this PR, such as linking more documentation pages for vectors.

@aaronfranke aaronfranke added enhancement documentation cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Oct 2, 2020
@aaronfranke aaronfranke added this to the 4.0 milestone Oct 2, 2020
@aaronfranke aaronfranke requested a review from a team as a code owner October 2, 2020 04:06
@akien-mga
Copy link
Member

akien-mga commented Oct 2, 2020

Given the amount of work done here I don't want to nitpick about casing, but we might need to define a guideline for what case style to use for link titles.

Current examples from existing and added titles:

  • Advanced vector math (Sentence case)
  • 2D Sprite animation (Sentence case, but note Sprite is the Node name here, not the English noun - though it's debatable whether it shouldn't be the noun, especially since it's renamed Sprite2D in 4.0)
  • 2D Dodge The Creeps Demo (Start Case, or invalid use of AP/Chicago style Title Case, as "the" should be lowercase)
  • GUI in 3D Demo (Title Case)

IMO that can be fixed in a follow-up PR reviewing the output of rg -g'*.xml' "link title=", after we've agreed on what style should be used consistently.

As always I'm strongly biased towards using Sentence case / against Title Case, which is the "consensus" that I somewhat imposed on godot-docs :)
But at the same time Juan's use of Title Case in the Godot editor labels has also been elevated to consensual, so it's not Manichean.

@akien-mga
Copy link
Member

akien-mga commented Oct 2, 2020

I'll merge as is anyway as it's a huge amount of work and already good as is. As mentioned above we can refine titles further once there's a consensus on style.

@akien-mga akien-mga merged commit 48afdfd into godotengine:master Oct 2, 2020
@akien-mga
Copy link
Member

Thanks!

@aaronfranke
Copy link
Member Author

@akien-mga For the demo links I used the same names as on the asset library, and for the documentation links I used the same names as the documentation articles. At some point we could have a discussion on style.

@aaronfranke aaronfranke deleted the doc-demo-links branch October 2, 2020 07:21
@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

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

2 participants