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

adding relaxed type-equivalency to ReParameter() #1639

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

cmstein
Copy link
Collaborator

@cmstein cmstein commented Jan 23, 2023

Signed-off-by: Clifford Stein cstein@imageworks.com

Description

Allows ShadingSystem->ReParameter() to use the same relaxed rules as SS->Parameter()

Tests

no changes in tests

Checklist:

  • I have read the contribution guidelines.
  • I have previously submitted a Contributor License Agreement.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the one comment and it needs clang-format.

@@ -404,6 +404,7 @@ class TypeSpec {
bool m_closure; ///< Is it a closure? (m_simple also used)
};

bool relaxed_equivalent(const TypeDesc& a, const TypeDesc& b);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this above to right after 'equivalent' and declare as a friend.

@lgritz
Copy link
Collaborator

lgritz commented Jan 25, 2023

@cmstein, can you please fix the clang-format (a line each in typedesc.cpp and shadingsys.cpp) and also edit the last commit you have to be sure it has the "Signed-off-by:" message for the DCO? Thanks.

I tried to do this myself, but for some reason can't seem to push it back to your branch (despite the message saying "Add more commits by pushing to the relaxed_equiv branch on cmstein/OpenShadingLanguage.", which I should be able to do as the maintainer of the project. I'm not sure why this doesn't work.

Signed-off-by: Clifford Stein <cstein@imageworks.com>
@cmstein
Copy link
Collaborator Author

cmstein commented Jan 25, 2023

fixed clang-format issues and squashed the commits

@lgritz lgritz merged commit c0a5998 into AcademySoftwareFoundation:main Jan 25, 2023
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Jan 28, 2023
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.

None yet

2 participants