You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "JosiahWI (via GitHub)" <gi...@apache.org> on 2023/10/13 13:46:17 UTC

[PR] Move CacheVC struct and methods from Vol [trafficserver]

JosiahWI opened a new pull request, #10601:
URL: https://github.com/apache/trafficserver/pull/10601

   This is part of the follow-up requested by @masaori335 in #10599. The remaining work is to deal with the methods defined in CacheRead.cc and CacheWrite.cc.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Move CacheVC struct and methods from Vol [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #10601:
URL: https://github.com/apache/trafficserver/pull/10601#issuecomment-1763633604

   Thanks for addressing the issue.
   
   For CacheRead.cc and CacheWrite.cc, we need to figure out what we want. Current code layout makes it clear which functions are related to reading or writing cache.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Move CacheVC struct and methods from Vol [trafficserver]

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #10601:
URL: https://github.com/apache/trafficserver/pull/10601#discussion_r1358385088


##########
iocore/cache/CacheVC.cc:
##########
@@ -21,12 +21,17 @@
   limitations under the License.
  */
 
+#include "I_Cache.h"
 #include "I_CacheDefs.h"
 #include "P_CacheDisk.h"
 #include "P_CacheHttp.h"
+// #include "P_CacheHosting.h"

Review Comment:
   delete this line.



##########
iocore/cache/CacheVC.cc:
##########
@@ -21,12 +21,17 @@
   limitations under the License.
  */
 
+#include "I_Cache.h"
 #include "I_CacheDefs.h"
 #include "P_CacheDisk.h"
 #include "P_CacheHttp.h"
+// #include "P_CacheHosting.h"
 #include "P_CacheInternal.h"
 #include "P_CacheVol.h"
 
+// must be included after the others

Review Comment:
   Should CacheVC.h include the required files itself?



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Move CacheVC struct and methods from Vol [trafficserver]

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #10601:
URL: https://github.com/apache/trafficserver/pull/10601


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Move CacheVC struct and methods from Vol [trafficserver]

Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #10601:
URL: https://github.com/apache/trafficserver/pull/10601#discussion_r1358388044


##########
iocore/cache/CacheVC.cc:
##########
@@ -21,12 +21,17 @@
   limitations under the License.
  */
 
+#include "I_Cache.h"
 #include "I_CacheDefs.h"
 #include "P_CacheDisk.h"
 #include "P_CacheHttp.h"
+// #include "P_CacheHosting.h"
 #include "P_CacheInternal.h"
 #include "P_CacheVol.h"
 
+// must be included after the others

Review Comment:
   I couldn't get that to work. I spent significantly more time trying to get that to work than I did making the change.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Move CacheVC struct and methods from Vol [trafficserver]

Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #10601:
URL: https://github.com/apache/trafficserver/pull/10601#issuecomment-1763666086

   Agreed. I think we would be losing something by doing away with that structure, but maybe we can come up with a better way to organize it.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org