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

Docs: update dispose return type #22237

Merged
merged 5 commits into from
Aug 3, 2021
Merged

Conversation

servinlp
Copy link
Contributor

@servinlp servinlp commented Aug 1, 2021

Description

On multiple places change the documentation for the dispose() method to return null

@mrdoob mrdoob added this to the r132 milestone Aug 2, 2021
@mrdoob
Copy link
Owner

mrdoob commented Aug 2, 2021

I think it should be [method:undefined dispose].

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 2, 2021

Then it's best if the OP corrects the other methods, too. null is used for all void helper methods right now.

@mrdoob
Copy link
Owner

mrdoob commented Aug 2, 2021

Oh... I guess we have many incorrect ones like that?

@servinlp
Copy link
Contributor Author

servinlp commented Aug 2, 2021

Ye method:null is currently used on quite a few places. Should all void methods have method:undefined as there return type? Like for example OrthographicCamera.setViewOffset (the code)

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 2, 2021

Oh... I guess we have many incorrect ones like that?

Yes, the docs are currently not consistent regarding returns types. If a method has not return statement, you can find the following approaches in the docs:

I think I vote for using void since this type is also used in TS.

@servinlp
Copy link
Contributor Author

servinlp commented Aug 2, 2021

void sound logical from a TS point of view however from having a quick look around it seems that method:null is most used right now.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 3, 2021

Before using null it really should be undefined.

@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2021

These methods do not return null. Either void or undefined works for me.

@servinlp
Copy link
Contributor Author

servinlp commented Aug 3, 2021

So void? I'm assuming we would want to do this them throughout the entire docs in the future aswell?

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 3, 2021

Then let's use undefined 👍.

@servinlp Are you willing to update the docs accordingly?

@servinlp
Copy link
Contributor Author

servinlp commented Aug 3, 2021

I will update this PR first and then see if I can change the rest in a different PR.

Do we have a document or something were we define documentation guide lines?

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 3, 2021

Do we have a document or something were we define documentation guide lines?

No, if possible I would like to avoid that and just correct the existing doc pages. If everything is consistent, copy/paste errors like before can be avoided.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 3, 2021

Do you mind updating the Chinese pages, too?

@mrdoob mrdoob changed the title feat: update doc for dispose return type Docs: update dispose return type Aug 3, 2021
@servinlp
Copy link
Contributor Author

servinlp commented Aug 3, 2021

It looks like the Chinese pages don't have the documentation for the dispose method yet

@mrdoob mrdoob merged commit dceadef into mrdoob:dev Aug 3, 2021
@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants