bzp2010 commented on code in PR #11403:
URL: https://github.com/apache/apisix/pull/11403#discussion_r1682312076


##########
t/plugin/authz-casdoor.t:
##########
@@ -105,10 +105,43 @@ __DATA__
     }
 --- response_body
 done
+--- error_log

Review Comment:
   I mean, if such is better (for reasons of complexity of modification now and 
on long term maintainability) and there is no such file now, we can create such 
a file and place the tests in one place.
   
   Do you think this has advantages for long term maintenance? The problem I 
can imagine is that plugin contributors need to know that their tests need to 
be written in multiple test files to test different parts. This requires either 
contributor knowledge or a careful reviewer.
   
   If so, let's move those tests to a new file.
   
   and cc @moonming @membphis for advice



##########
apisix/plugins/authz-keycloak.lua:
##########
@@ -114,6 +114,11 @@ local _M = {
 
 
 function _M.check_schema(conf)
+    local check = {"discovery", "token_endpoint", 
"resource_registration_endpoint",
+                    "access_denied_redirect_uri"}
+    core.utils.check_https(check, conf, plugin_name)
+    core.utils.check_tls_bool({conf.ssl_verify}, {"ssl_verify"}, plugin_name)

Review Comment:
   Just simply curious, why is the style of the two check functions so 
inconsistent? 🤔



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to