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

chore: unify logic for metrics collection when any command is executed #8

Merged
merged 8 commits into from
Jan 12, 2024

Conversation

peter-rr
Copy link
Collaborator

@peter-rr peter-rr commented Dec 20, 2023

Add new methods to unify logic so metrics are collected when any CLI command is executed. All this logic will be implemented in base.ts file and removed from the different command files, except the metadata generated for every different command.

@peter-rr
Copy link
Collaborator Author

peter-rr commented Dec 20, 2023

@smoya Only applied to validate command. Still needs to be applied to the rest of commands as well.

@peter-rr peter-rr changed the title Unify logic for metrics collection when any command is executed feat: unify logic for metrics collection when any command is executed Dec 20, 2023
@peter-rr
Copy link
Collaborator Author

@smoya Logic applied to the rest of commands already (just the five ones covered in this POC).

@peter-rr peter-rr requested a review from smoya December 21, 2023 12:30
src/base.ts Outdated Show resolved Hide resolved
@peter-rr peter-rr requested a review from smoya January 10, 2024 12:40
src/commands/bundle.ts Outdated Show resolved Hide resolved
@@ -78,13 +78,13 @@ export default class Convert extends Command {
if (document !== undefined) {
metadata = MetadataFromDocument(document, metadata);
metadata['from_version'] = document.version();
this.specFile = await load(filePath);
Copy link
Owner

Choose a reason for hiding this comment

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

I'm concerned about this extra parsing stuff here. Would it make sense to use the original file (specFile var) instead 🤔

Copy link
Owner

Choose a reason for hiding this comment

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

Nevermind, we can revisit all of that in the general PR.

@@ -14,7 +14,7 @@ const { writeFile } = promises;
export enum Optimizations {
REMOVE_COMPONENTS='remove-components',
REUSE_COMPONENTS='reuse-components',
MOVE_TO_COMPONETS='move-to-components'
MOVE_TO_COMPONENTS='move-to-components'
Copy link
Owner

Choose a reason for hiding this comment

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

john-cena-deformed-gif

Copy link
Collaborator Author

@peter-rr peter-rr Jan 10, 2024

Choose a reason for hiding this comment

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

Yeah...it was there since a loooong time ago 🙈

@smoya smoya changed the title feat: unify logic for metrics collection when any command is executed chore: unify logic for metrics collection when any command is executed Jan 10, 2024
@smoya
Copy link
Owner

smoya commented Jan 10, 2024

Changed the title to chore as it is basically a refactor not adding any feature

@smoya smoya merged commit d57655b into smoya:feat/adoptionMetrics Jan 12, 2024
5 of 8 checks passed
@peter-rr peter-rr deleted the feat/adoptMetrics-v4 branch February 2, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants