You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/06/25 09:51:30 UTC
[GitHub] [apisix] Yiyiyimu opened a new pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Yiyiyimu opened a new pull request #4484:
URL: https://github.com/apache/apisix/pull/4484
Signed-off-by: yiyiyimu <wo...@gmail.com>
### What this PR does / why we need it:
fix #4449
And also remove unnecessary quotes in config. Check the feasibility with different special characters in https://github.com/api7/lua-tinyyaml/pull/6
But not sure if it's necessary. I'll revert it if no need to do so.
### Pre-submission checklist:
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4484:
URL: https://github.com/apache/apisix/pull/4484
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r659260464
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Have you added test to verify if this change works?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r660831023
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Got it. Test added.
I guess releasing new versions for tiny patches could be troublesome 😿 If it's not of course it would be great
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r660264492
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Err, actually I mean testing with `disk_path` only. We can check if the default generated conf has this field.
BTW, why not release the rock first?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r660258088
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Tested in yaml repo: https://github.com/Yiyiyimu/lua-tinyyaml/blob/test/symbols/spec/map_spec.lua#L57-L64 Do we need tests also in this repo?
BTW, I fix the bug causing the CI failure in that PR but I'm not sure if there are any other bugs. Is there any method to run CI with un-released lua rocks? Or should I create a temporary rock for the test, and if everything goes correctly, then we make the new release?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r660258088
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Tested in yaml repo: https://github.com/Yiyiyimu/lua-tinyyaml/blob/test/symbols/spec/map_spec.lua#L57-L64 Do we need tests also in this repo?
BTW, I fix the bug causing the CI failure in that PR but I'm not sure if there are any other bugs. Is there any method to run CI with un-released lua rocks? Or should I create a temporary rock for the test, and if everything goes correctly, then we make the new release?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4484: chore: update tinyyaml lib and remove unnecessary quotes in config
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4484:
URL: https://github.com/apache/apisix/pull/4484#discussion_r660264492
##########
File path: conf/config-default.yaml
##########
@@ -52,8 +52,8 @@ apisix:
# which cache to use by name in the admin api
memory_size: 50m # The size of shared memory, it's used to store the cache index
disk_size: 1G # The size of disk, it's used to store the cache data
- disk_path: "/tmp/disk_cache_one" # The path to store the cache data
Review comment:
Err, actually I mean testing with `disk_path` only. We can check if the default generated conf has this field.
BTW, why not release the rock first?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org