Rename shaderc_spv_module to shaderc_compilation_result #126
Conversation
|
We still have a lot of C API function named like: shaderc_module_*. Do we need to modify them, as we don't have a type named shaderc_spv_module anymore? |
Qining
force-pushed
the
rename-shaderc_spv_module-to-shaderc_compilation_result
branch
from
92ae3f7
to
523062d
Compare
| void shaderc_module_release(shaderc_spv_module_t module); | ||
| // Releases the resources held by the result module. It is invalid to use the | ||
| // module for any further operations. | ||
| void shaderc_module_release(shaderc_compilation_result_t module); |
There was a problem hiding this comment.
I find module a bit confusing now that it's unrelated to the type name. Perhaps we can rename it to result?
There was a problem hiding this comment.
+1
So shaderc_result_release?
The primary job of shaderc is to produce a compilation result, so there should be no problem interpreting "result" in this case.
There was a problem hiding this comment.
Yes, plus name the parameter result and adjust the blurb accordingly.
|
I would change the function names, too. Otherwise they'll send confusing signals to the reader. |
| @@ -93,7 +94,7 @@ class SpvModule { | |||
| SpvModule(const SpvModule& other) = delete; | |||
| SpvModule& operator=(const SpvModule& other) = delete; | |||
|
|
|||
| shaderc_spv_module_t module_; | |||
| shaderc_compilation_result_t module_; | |||
There was a problem hiding this comment.
rename module_ to compilation_result_
|
-1 |
Qining
force-pushed
the
rename-shaderc_spv_module-to-shaderc_compilation_result
branch
from
523062d
to
d125ce4
Compare
|
Updated both C and CPP API. I guess the word "result module' in comments should be fine? |
Nope, it's equally confusing in comments as it was in the names. :) |
Qining
force-pushed
the
rename-shaderc_spv_module-to-shaderc_compilation_result
branch
from
d125ce4
to
1c21d70
Compare
|
Replace 'result module' in comments with 'result object'. |
| // further operations. | ||
| void shaderc_module_release(shaderc_spv_module_t module); | ||
| // Releases the resources held by the result object. It is invalid to use the | ||
| // the data held by the result object for any further operations. |
There was a problem hiding this comment.
Typo: "the the". Also, I'd remove the entire phrase "the data held by" and keep just "the result object".
Qining
force-pushed
the
rename-shaderc_spv_module-to-shaderc_compilation_result
branch
4 times, most recently
from
7014c25
to
14465d4
Compare
Rename the result object from shaderc_spv_module to shaderc_compilation_result. Also rename the involved methods, variables, and handlers. Rename an enum status: shaderc_compilation_status_null_result_module to shaderc_compilation_status_null_result_object.
Qining
force-pushed
the
rename-shaderc_spv_module-to-shaderc_compilation_result
branch
from
14465d4
to
763eff0
Compare
|
+2 |
No description provided.