Skip to content

Conversation

samxbr
Copy link
Contributor

@samxbr samxbr commented Aug 7, 2025

Rename a bunch of transport actions from XXXTransportAction to TransportXXXAction so they are more consistent with the rest of the transport actions.

@samxbr samxbr added >refactoring auto-backport Automatically create backport pull requests when merged v9.2.0 v8.19.2 v9.1.2 v8.18.6 v9.0.6 labels Aug 7, 2025
@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Aug 7, 2025
@rjernst
Copy link
Member

rjernst commented Aug 7, 2025

This seems the wrong direction to me. XXXTransportAction is more canonical in my mind, and parallels how rest actions (should be) named, as XXXRestAction (or XXXRestHandler, but there's a whole other set of naming inconsistencies there to consider).

@samxbr
Copy link
Contributor Author

samxbr commented Aug 7, 2025

This seems the wrong direction to me. XXXTransportAction is more canonical in my mind, and parallels how rest actions (should be) named, as XXXRestAction (or XXXRestHandler, but there's a whole other set of naming inconsistencies there to consider).

Naming is always the hard topic... To me it seems TransportXXXAction and RestXXXAction are used more widely than the other way. I think I have seem TransportXXXAction/RestXXXAction more frequently than XXXTransportAction/XXXRestAction, which made me thought they are accepted more widely, but I could be wrong on this. The intention is to make these names a little bit more standardized across, it always bugs me that I need to try different names when searching these actions.

(Personally I feel it is a tiny bit more readable to see the keyword Transport/Rest at the front)

@samxbr samxbr marked this pull request as ready for review August 8, 2025 15:17
@samxbr samxbr requested a review from a team as a code owner August 8, 2025 15:17
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Aug 8, 2025
@samxbr samxbr added the :Core/Infra/Transport API Transport client API label Aug 8, 2025
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team and removed needs:triage Requires assignment of a team area label labels Aug 8, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@samxbr
Copy link
Contributor Author

samxbr commented Aug 8, 2025

There are some more transport actions with inconsistent name but I am going to stop here and put this out to get some opinion first.

@samxbr
Copy link
Contributor Author

samxbr commented Aug 8, 2025

There are different options on this naming convention:

  1. leave it the way it is (mixed XXXTransportAction and TransportXXXAction)
  2. choose a naming convention and consolidate them (XXXTransportAction vs TransportXXXAction, or maybe other options)

I'd like to do 2) and personally I am in favor of TransportXXXAction. @rjernst Since you express interest, do you think this needs more discussion, or maybe with more audience?

@rjernst
Copy link
Member

rjernst commented Aug 8, 2025

I think it will need some discussion. Github comments probably aren't the right place for that discussion.

@samxbr
Copy link
Contributor Author

samxbr commented Aug 8, 2025

Closing this PR for now after discussion with @rjernst. There's more discussion needed and a decision needs to be made within Core Infra. Opened an issue instead.

@samxbr samxbr closed this Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Transport API Transport client API >refactoring serverless-linked Added by automation, don't add manually Team:Core/Infra Meta label for core/infra team v8.18.6 v8.19.2 v9.0.6 v9.1.2 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants