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

add editors breakdown #24

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

giotab
Copy link

@giotab giotab commented Apr 30, 2024

In order to see Editors usage as well, I changed the existing LanguagesBreakdown component to be more generic so that both Languages and Editors require less effort to maintain.

github copilot editors pie chart

Here are the most significant changes:

New Features:

  • src/components/BreakdownsComponent.vue: Introduced a new component that provides a generic breakdown view. This component can display breakdowns by different keys such as language or editor. It includes a pie chart to visualize the top 5 items by accepted prompts and acceptance rate, and a data table for a detailed breakdown.

Enhancements:

Refactoring:

  • src/model/Breakdown.ts: Renamed from Language.ts and updated the class name to Breakdown to make it more generic.
  • src/model/Metrics.ts: Renamed the Breakdown class to BreakdownData and updated the breakdown member of the Metrics class to use BreakdownData. [1] [2] [3]

Removals:

@martedesco martedesco self-requested a review May 2, 2024 17:48
@martedesco
Copy link
Collaborator

@giotab , thanks for your contribution! I added a few comments 👍🏼

@giotab
Copy link
Author

giotab commented May 11, 2024

@giotab , thanks for your contribution! I added a few comments 👍🏼

Hi @martedesco , not sure I can see the comments 🤔 unless you mean in the PR's description?
Also, should I bump the version number to 1.5.0?

@@ -0,0 +1,243 @@
<template>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to name it BreakdownComponent.vue? As I would assume it is a singular breakdown per component e.g. languages or editors

<v-card-item>
<div>
<div class="text-overline mb-1" style="visibility: hidden;">filler</div>
<div class="text-h6 mb-1">Number of {{ breakdownKey }}s</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<div class="text-h6 mb-1">Number of {{ breakdownKey }}s</div>
<div class="text-h6 mb-1">Number of {{ breakdownKey }}</div>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a specific constraint to use the keys on the singular? If we use Editors and Languages as key we wouldn't need to add 's' in every time we reference this prop.

Copy link
Author

Choose a reason for hiding this comment

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

The breakdownKey is used in this line
const breakdownName = breakdownData[props.breakdownKey as keyof typeof breakdownData] as string;
in order to get the value, e.g. breakdownName would be dockerfile or vscode.

{
    "language": "dockerfile",
    "editor": "vscode",
    "suggestions_count": 3,
    "acceptances_count": 0,
    "lines_suggested": 3,
    "lines_accepted": 0,
    "active_users": 1
}

(replaces the previous const languageName = breakdown.language;)

If we use Editors, then the above access will not work.

I updated the code a little bit, to centralise the "pluralisation" of the key.

src/components/MetricsViewer.vue Show resolved Hide resolved
@martedesco
Copy link
Collaborator

Hi @martedesco , not sure I can see the comments 🤔 unless you mean in the PR's description?

My bad, I didn't submit them!

Also, should I bump the version number to 1.5.0?

I will add a few things before releasing it, so no need to worry about that 🙏🏼

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.

None yet

2 participants