Skip to content

Conversation

@cmcfarlen
Copy link
Contributor

This PR moves the code depending on iocore/eventsystem into eventsystem to avoid circular dependencies. I'm open to better naming for files and functions or a better method for encapsulating the event system code, but this does remove the circular deps.

@cmcfarlen cmcfarlen self-assigned this Mar 8, 2023
@cmcfarlen cmcfarlen added this to the 10.0.0 milestone Mar 8, 2023
@cmcfarlen cmcfarlen force-pushed the fix-records-events-deps branch from 6fb59e9 to 0d7a84c Compare March 9, 2023 22:26
@cmcfarlen cmcfarlen requested a review from brbzull0 March 10, 2023 16:32
brbzull0
brbzull0 previously approved these changes Mar 13, 2023
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a minor typo.


// This defines the interface to the low level stat block operations
// The implementation of this was moved out of the records library due to a circular dependency this produced.
// look for the implmenetation of RecRawStatBlockOps in iocore/eventsystem
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo? implementation

Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@bryancall bryancall self-requested a review March 13, 2023 22:09
#include "I_Machine.h"
#include "records/I_RecordsConfig.h"
#include "records/I_RecProcess.h"
#include "RecProcess.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since RecProcess.h is an header file used by other subsystems, shouldn't it be in the include/ directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but there isn't an include/iocore directory and everything that depends on this already has iocore/eventsystem in their include path.

@cmcfarlen cmcfarlen merged commit 8b8145f into apache:master Mar 22, 2023
@cmcfarlen cmcfarlen deleted the fix-records-events-deps branch March 22, 2023 19:43
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master: (42 commits)
  Add logic to make the server.policy and server.properties settings reloadable (apache#9572)
  Add CMake to the required PR CI builds (apache#9575)
  fixup cmake build for master and add conditional for io_uring support (apache#9571)
  Cleanup: Use swoc::meta instead of ts::meta. (apache#9566)
  codeql 24: Multiplication result converted to larger type (apache#9569)
  Drop support for old quiche (apache#9561)
  QUIC: Ignore default_inactivity_timeout in favour of proxy.config.quic.no_activity_timeout_in. (apache#9564)
  Fix log format specifications (apache#9568)
  Add `current_time_epoch_ms` stat to be appended before the server version. This allows computation of stats externally based on the cache time frame. This can help alleviate issues with sliding windows between various stats programs that generate discrepencies (apache#9567)
  Define BIO macros in ink_ssl.h (apache#9557)
  combine UDPPacket and UDPPacketInternal (apache#9424)
  Update codeql.yml (apache#9560)
  Http2 to origin (apache#9366)
  coverity 1497413: Use of 32-bit time_t (apache#9556)
  Add support for multiple yaml config files for wasm plugin (apache#9483)
  Add TS_HAS_QUICHE feature variable. (apache#9547)
  mime header field parsing fix trailing quote handlling (apache#9513)
  Make magick plugin buildable with BoringSSL (apache#9554)
  QUIC: Test basic scenarios around the ts.quic.no_activity_timeout_in config. (apache#9543)
  Fix records events deps (apache#9511)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants