Skip to content

Conversation

@Oletus
Copy link
Contributor

@Oletus Oletus commented Oct 17, 2019

This name is clearer since "code" can be easily confused to mean shader code in this context.

Settling on a good name here is important because we want to expose the ability to customize the hash through the API. ( #17567 )

@Oletus
Copy link
Contributor Author

Oletus commented Oct 17, 2019

@Mugen87 please review, I think it would be important to clarify the naming before we put this in the API ( #17567 ).

@Oletus Oletus force-pushed the rename-program-code branch from fc806ba to f964ded Compare October 17, 2019 07:10
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 17, 2019

This name is clearer since "code" can be easily confused to mean shader code in this context.

I know what you mean but the term "hash" would better fit with the following implementation #16674. The PR computes a CRC32 value which is something that can be considered as an actual hash value.

I think I vote for programCacheKey what you have already suggested in #17567.

This name is clearer since "code" can be easily confused to mean shader code in this context.

Settling on a good name here is important because we want to expose the ability to customize the cache key through the API.
@Oletus Oletus force-pushed the rename-program-code branch from f964ded to 91d02e7 Compare October 17, 2019 09:50
@Oletus
Copy link
Contributor Author

Oletus commented Oct 17, 2019

Cool, I'm happy with "cacheKey". Pushed an amended commit changing the name to cacheKey instead.

@Mugen87 Mugen87 changed the title Rename WebGLProgram "code" to "hash" Rename WebGLProgram "code" to "cacheKey" Oct 17, 2019
@Oletus
Copy link
Contributor Author

Oletus commented Oct 17, 2019

@mrdoob Please merge!

@Mugen87 Mugen87 added this to the r110 milestone Oct 18, 2019
@mrdoob mrdoob merged commit a6b2e37 into mrdoob:dev Oct 21, 2019
@mrdoob
Copy link
Owner

mrdoob commented Oct 21, 2019

Thanks!

@Oletus Oletus deleted the rename-program-code branch October 21, 2019 21:16
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