Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: naming convention of core utilities #26

Closed
saragerion opened this issue Apr 7, 2021 · 10 comments · Fixed by #102 or #107
Closed

RFC: naming convention of core utilities #26

saragerion opened this issue Apr 7, 2021 · 10 comments · Fixed by #102 or #107
Assignees
Labels
completed This item is complete and has been merged/shipped RFC Technical design documents related to a feature request
Milestone

Comments

@saragerion
Copy link
Contributor

saragerion commented Apr 7, 2021

Description of the proposal

We need to come to an agreement about the naming convention of the core utilities, to keep them consistent within the wider TypeScript tools.

Name of the core utility (and consequently the folders that contain the libraries):

  • packages/logger, packages/tracer, packages/meter
    OR
  • packages/logs, packages/traces, packages/metrics
    OR
  • packages/logging, packages/tracing, packages/metrics (?)

Name of Class that is used in the code to call the library:

  • const logger = new Logger();, const tracer = new Tracer();, const meter = new Meter();
    OR
  • const logs = new Logs();, const traces = new Traces();, const metrics = new Metrics();
    OR
  • const logging = new Logging();, const tracing = new Tracing();, const metrics = new Metrics(); (?)

Related issues, RFCs

TODO: this needs to be created/moved in the official RFC repo: https://github.com/awslabs/aws-lambda-powertools-rfcs.
Adding this issue here for visibility and to open the conversation.

@saragerion saragerion changed the title RFC: naming convention RFC: naming convention of core utilities Apr 7, 2021
@saragerion saragerion added this to the beta-release milestone Apr 7, 2021
@saragerion saragerion added this to Needs triage / RFC / clarification in Issues via automation Apr 7, 2021
@heitorlessa
Copy link
Contributor

Option C and A

packages/logging, packages/tracing, packages/metrics

Logger, Tracer, Metrics

These make it easier to import, same as Python, and easier on docs too ;)

Suuuuper excited and looking forward to this

@saragerion saragerion moved this from Needs triage / RFC / clarification to To do (triaged) in Issues Jun 10, 2021
@stale
Copy link

stale bot commented Jun 11, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the pending-close-response-required This issue will be closed soon unless the discussion moves forward label Jun 11, 2021
@saragerion saragerion removed the pending-close-response-required This issue will be closed soon unless the discussion moves forward label Jul 16, 2021
@saragerion
Copy link
Contributor Author

@alan-churley @dreamorosi what's your thought on this one? Feel free to leave a comment with your preference

This was linked to pull requests Jul 16, 2021
@dreamorosi
Copy link
Contributor

dreamorosi commented Jul 16, 2021

I like C and A as well.

The package describes the action/service (logging, tracing, metrics) while the class reads like "this is an instance of a Logger|Tracer|Meter".

The problem with this is that currently the Python Powertools use Metrics and not Meter so we'd have to rename and this could cause confusion. A solution if we like this convention could be to have an alias only in Python and for historical reasons for a while so that we wouldn't break any customer implementation.

@heitorlessa
Copy link
Contributor

Sorry I read A too quickly and didn't see Meter in it --- tbh, I wouldn't be too worried about having Tracer, Logger, and Metrics --- there are going to be other exceptions as other utilities get created

@dreamorosi
Copy link
Contributor

Don't worry and agree with you about the exception. From a language point of view it makes sense for it to be different since contrary to logging and tracing it doesn't express a verb.

Happy to hear and discuss other opinions tho.

@heitorlessa
Copy link
Contributor

heitorlessa commented Jul 16, 2021 via email

@bahrmichael
Copy link
Contributor

Without reading the previous comments, I'm in favor of Option C. I was initially for A, but "meter" sounds less clear to me than "metrics". Also "inch" vs. "meter" jokes. Not inclined for Option B, as those sound more like collections of utilities. I understand this to be more of a one class, that helps me do a bunch of things, rather than many functions (per package) that I can pick for various use cases.


Input from my team:

I think it depends on what the thing actually is. Logger sounds like a single class/module responsible for logging things, while Logging or Logs is rather a collection of such classes/modules.


After reading the comments above, I'm leaning towards a mix and match to whatever our customers are already used to. I'd prefer little cognitive complexity when switching back and forth between languages and their Lambda powertools.


Summary: Inclined towards making it easy for customers by staying consistent with other powertools libraries.

@simontabor
Copy link

Personal preference is packages/logger, packages/tracer, packages/metrics

Semantics should come second to usability here IMO, and the added perk of being consistent with Python powertools

@saragerion saragerion moved this from To do (triaged) to In progress in Issues Jul 20, 2021
Issues automation moved this from Issues - In progress (triaged) to Issues - Done (closed) Aug 11, 2021
@github-actions
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@saragerion saragerion moved this from Issues - Done (closed) to Issues - Done (released) in Issues Jan 5, 2022
@dreamorosi dreamorosi removed this from Issues - Done (released) in Issues Nov 13, 2022
@dreamorosi dreamorosi added RFC Technical design documents related to a feature request completed This item is complete and has been merged/shipped labels Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped RFC Technical design documents related to a feature request
Projects
None yet
8 participants