You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "lzx404243 (via GitHub)" <gi...@apache.org> on 2023/10/04 15:46:32 UTC
[PR] Move mgmt/ and iocore/ files to include/ and src/ [trafficserver]
lzx404243 opened a new pull request, #10558:
URL: https://github.com/apache/trafficserver/pull/10558
As titled.
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen closed pull request #10558: Move mgmt/ and iocore/ files to include/ and src/
URL: https://github.com/apache/trafficserver/pull/10558
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "dragon512 (via GitHub)" <gi...@apache.org>.
dragon512 commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1767186253
I don't have any disagreement with the change with P_ or I_ values. However this PR was about moving the source as that is a big move, then making change to the source files as PR after the move. I am concerned about trying to move and change at the same time with a lot of "smaller" PRs on in that the small PRs will be bigger than we want because of the "ball of Wax" issues that exists with the current headers/source files. @zwoop is concerned as I understand about making cherry picks, and the point of having all the source file move in one PR vs many was to make the possibility of moving source in an updated 9 easier than harder. At least that is the intent here.
I feel we need to get the main sources files moved then clean up the smaller items such as the which headers go where or renames of files. I don't want "the perfect to be the enemy of the good" here
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1764783087
I agree with everything Chris said. :-).
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1767430535
Maybe Just have a couple of PRs then?
1. Move the directories
2. Move the I_* headers but drop the I_ part
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1764701612
This looks OK to me except for moving the `P_.*` files. These are private headers that would have been in `.cc` files were it not for the odd "redefine and include" trick with the `Inline.cc` files. I know the `P_` headers were moved into `include/` for the records library, but I think we should talk about whether or not we want those in the public include directory for 10.0 or if they should stay an implementation detail in the `src` directory.
Further, I think we should drop the `I_` prefix for files we move to `include/` since the rest of the includes don't have that prefix.
For me, ideally, we would move the top level source directories (iocore, mgmt, proxy, maybe plugins) into src first, and then cleanup and move the headers in smaller PRs.
I'm not entirely against merging this PR as is, but we should decide what we want to put in include before the next release branch.
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1764803144
Also, with this landed, everyone better be ready and prepared to make branch PRs, because cherry-picking from master to 9.x is not going to be trivial anymore. :)
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "lzx404243 (via GitHub)" <gi...@apache.org>.
lzx404243 commented on PR #10558:
URL: https://github.com/apache/trafficserver/pull/10558#issuecomment-1771273437
This is replaced by https://github.com/apache/trafficserver/pull/10638
--
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 mgmt/ and iocore/ files to include/ and src/ [trafficserver]
Posted by "lzx404243 (via GitHub)" <gi...@apache.org>.
lzx404243 closed pull request #10558: Move mgmt/ and iocore/ files to include/ and src/
URL: https://github.com/apache/trafficserver/pull/10558
--
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