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.

Sign up for GitHub

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

Jump to bottom

Removed a non supported parameter from the docstring #23651

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Removed a non supported parameter from the docstring #23651

merged 1 commit into from
Mar 3, 2022

Conversation

deepansh96
Copy link
Contributor

Related issue: #XXXX

Description

The opacity parameter is not valid for LineBasicMaterial, it's also not mentioned in the documentation.
But this was mentioned in the parameters docstring, which VSCode also provides as a possible suggestion.
I've removed that line in this PR

@deepansh96
Removed a non supported parameter from the docstring
2422d4f 
The `opacity` parameter is not valid for LineBasicMaterial, it's also not mentioned in the documentation.
But this was mentioned in the parameters docstring, which VSCode also provides as a possible suggestion.
@mrdoob mrdoob added this to the r139 milestone Mar 3, 2022
@mrdoob mrdoob merged commit f3080e3 into mrdoob:dev Mar 3, 2022
@mrdoob
Copy link
Owner

mrdoob commented Mar 3, 2022

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Mar 3, 2022

We probably should remove all these docstring...

@deepansh96 deepansh96 deleted the patch-1 branch March 4, 2022 06:46
@Mugen87 Mugen87 mentioned this pull request Mar 5, 2022
Merged
@WestLangley
Copy link
Collaborator

@deepansh96

The opacity parameter is not valid for LineBasicMaterial

Yes it is.

it's also not mentioned in the documentation

It is mentioned in the docs for the base Material class.

Actually, I find this confusing for users. Why should users have to concern themselves with the inheritance structure of the three.js classes?

@mrdoob
Copy link
Owner

mrdoob commented Mar 8, 2022

Actually, I find this confusing for users. Why should users have to concern themselves with the inheritance structure of the three.js classes?

What do you mean?

@WestLangley
Copy link
Collaborator

The LineBasicMaterial docs list only some of the supported properties and methods, instead saying: "See the base Material class for common properties."

Apparently, that is why the OP was confused.

But I agree, it is probably overkill to list all the properties for each class.

donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
@deepansh96 @donmccurdy
Removed a non supported parameter from the docstring (mrdoob#23651)
6b7f4cf 
The `opacity` parameter is not valid for LineBasicMaterial, it's also not mentioned in the documentation.
But this was mentioned in the parameters docstring, which VSCode also provides as a possible suggestion.
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
@deepansh96 @abernier
Removed a non supported parameter from the docstring (mrdoob#23651)
7cae193 
The `opacity` parameter is not valid for LineBasicMaterial, it's also not mentioned in the documentation.
But this was mentioned in the parameters docstring, which VSCode also provides as a possible suggestion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r139
Development

Successfully merging this pull request may close these issues.
3 participants
@deepansh96 @mrdoob @WestLangley