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

feat: YoloChart in repo pages #3658

Merged
merged 22 commits into from
Jul 8, 2024
Merged

feat: YoloChart in repo pages #3658

merged 22 commits into from
Jul 8, 2024

Conversation

zeucapua
Copy link
Contributor

@zeucapua zeucapua commented Jul 1, 2024

Description

Implements the YoloChart in repo pages to show YOLO coders, contributors that push commits directly on the "main" branch.

Related Tickets & Documents

Closes #3644
Closes #3584

Mobile & Desktop Screenshots/Recordings

Screen.Recording.2024-07-05.at.10.32.19.mov

Steps to QA

Tier (staff will fill in)

  • Tier 1
  • Tier 2
  • Tier 3
  • Tier 4

[optional] What gif best describes this PR or how it makes you feel?

donald-duck-yolo-gif

Copy link

netlify bot commented Jul 1, 2024

Deploy Preview for design-insights ready!

Name Link
🔨 Latest commit 2add587
🔍 Latest deploy log https://app.netlify.com/sites/design-insights/deploys/668c1a9a2b3728000845e7c2
😎 Deploy Preview https://deploy-preview-3658--design-insights.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 1, 2024

Deploy Preview for oss-insights ready!

Name Link
🔨 Latest commit 2add587
🔍 Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/668c1a9a5ac72e0009ba45ba
😎 Deploy Preview https://deploy-preview-3658--oss-insights.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@zeucapua zeucapua marked this pull request as ready for review July 1, 2024 22:21
@zeucapua zeucapua requested a review from a team July 1, 2024 22:21
@bdougie
Copy link
Member

bdougie commented Jul 2, 2024

This is really cool to see live. I notice a lot more projects than expected have a link to YOLOs.

I.e. Facebook/react might be due to how they do merge queues. Not sure though.

image

We might to do a similar maintainer QA in the slack channel to get feedback on this.

Also, the link to why this would be a problem goes to the docs. Is there a docs issues for that?

@BekahHW
Copy link
Member

BekahHW commented Jul 2, 2024

@zeucapua I have the docs up for it, so as soon as it's reviewed, you can link to them.

open-sauced/docs#348

@brandonroberts
Copy link
Contributor

The pill is a little stretched when expanding the screen

image

Copy link
Member

@isabensusan isabensusan left a comment

Choose a reason for hiding this comment

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

Thank you for this @zeucapua !

  1. One extra comment for big screens:
  • let's add extra padding to make the button bigger so it doesn't look disproportionate (px-1 and py-1 should work)
  • can we add a "Show more" text next to the arrow to fill the button more and make it feel less empty?
image
  1. Can we add the little "YOLO Coder" icon to the Lottery factor table to signal yolo coders?
  • icon on hover should display the simple tooltip saying "YOLO coder"
  • icon on click should reveal the YOLO view
image
  1. This is probably an api side issue, but if we could remove bots from the response that'd be amazing.
image
  1. Can we get rid of the extra bottom border on the last item of the YOLO table? (and if possible, do the same for the lottery factor table)
image

components/Repositories/LotteryFactorChart.tsx Outdated Show resolved Hide resolved
components/Repositories/LotteryFactorChart.tsx Outdated Show resolved Hide resolved
components/Repositories/YoloChart.tsx Show resolved Hide resolved
Copy link
Member

@isabensusan isabensusan left a comment

Choose a reason for hiding this comment

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

see comment above

@jpmcb
Copy link
Member

jpmcb commented Jul 2, 2024

This is probably an api side issue, but if we could remove bots from the response that'd be amazing.

This is worth having a quick discussion on:

I believe it would be interesting for maintainers to see what bot are pushing directly to main, partially because bots (like in your example the k8s prow bot or the k8s test infra bot) make up a HUGE part of a project's secure software supply chain. I.e., bots (and the software they consume) are just another piece of the ever expanding puzzle that makes up building and shipping secure software.

Bots committing directly to main are a common practice (generally safer than a maintainer sending something directly to main and are useful in release processes) but can still be compromised: compromised in sometimes extremely catostrophic ways. The SolarWinds attack and data breach was as bad as it was because hacked build system dependencies (like bots or other pieces of build automation) went unnoticed for so long, malicious third party actors were able to inject back doors into many of their products. Then resulting in all their customers becoming compromised.

We can remove bot commits API side for this early iteration. But I'd be more supportive of a long term option that makes it on/off optional. Or we think over / design another piece of this component that incorporates bot commits directly to main.

@nickytonline
Copy link
Member

This is probably an api side issue, but if we could remove bots from the response that'd be amazing.

This is worth having a quick discussion on:

I believe it would be interesting for maintainers to see what bot are pushing directly to main, partially because bots (like in your example the k8s prow bot or the k8s test infra bot) make up a HUGE part of a project's secure software supply chain. I.e., bots (and the software they consume) are just another piece of the ever expanding puzzle that makes up building and shipping secure software.

Bots committing directly to main are a common practice (generally safer than a maintainer sending something directly to main and are useful in release processes) but can still be compromised: compromised in sometimes extremely catostrophic ways. The SolarWinds attack and data breach was as bad as it was because hacked build system dependencies (like bots or other pieces of build automation) went unnoticed for so long, malicious third party actors were able to inject back doors into many of their products. Then resulting in all their customers becoming compromised.

We can remove bot commits API side for this early iteration. But I'd be more supportive of a long term option that makes it on/off optional. Or we think over / design another piece of this component that incorporates bot commits directly to main.

I think a toggle for this is a good idea.

@zeucapua zeucapua requested a review from isabensusan July 2, 2024 21:12
Copy link
Contributor

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

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

I left some UI/UX feedback, but won't block this since it'd be good to get this out.

🚢

isabensusan

This comment was marked as duplicate.

Copy link
Member

@isabensusan isabensusan left a comment

Choose a reason for hiding this comment

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

left comments on responsiveness! I keep adding reviews as comments 🤦‍♀️

@isabensusan
Copy link
Member

@zeucapua for the bots toggle, here's two ideas:
I prefer the one on the right, but understand the one on the left is possibly easier to implement responsively
image

@zeucapua
Copy link
Contributor Author

zeucapua commented Jul 5, 2024

YOLO Banner breakpoint designs implemented cc @isabensusan

Screen.Recording.2024-07-05.at.10.20.12.mov

@zeucapua zeucapua requested a review from isabensusan July 5, 2024 17:40
@jpmcb
Copy link
Member

jpmcb commented Jul 5, 2024

FYI, quickly worth calling out, the bots toggle won't work for accounts that don't end in [bot]. So, the kubernetes bot accounts won't get filtered. I've opened this issue to explore a "well known bot account" service of some kind to start solving for this: https://github.com/open-sauced/api/issues/929

Copy link
Member

@isabensusan isabensusan left a comment

Choose a reason for hiding this comment

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

@zeucapua looking SO much better now!!!! Thank you for the extra effort

It looks great in basically every breakpoint, except between 1440px - 1650px where it breaks, which unfortunately is exactly the default breakpoint for 13 inch macbooks 😢

If we can make sure that it looks nice at 1512px, then this is good to ship!

image

@zeucapua
Copy link
Contributor Author

zeucapua commented Jul 8, 2024

Copy link
Member

@isabensusan isabensusan left a comment

Choose a reason for hiding this comment

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

Thank you!!! Can confirm it now looks perfect :)

@zeucapua zeucapua merged commit 9462519 into beta Jul 8, 2024
13 checks passed
@zeucapua zeucapua deleted the feat/yolo-lottery-factor branch July 8, 2024 21:06
open-sauced bot pushed a commit that referenced this pull request Jul 8, 2024
## [2.43.0-beta.1](v2.42.1-beta.2...v2.43.0-beta.1) (2024-07-08)

### 🍕 Features

* `YoloChart` in repo pages ([#3658](#3658)) ([9462519](9462519))
open-sauced bot pushed a commit that referenced this pull request Jul 9, 2024
## [2.43.0](v2.42.0...v2.43.0) (2024-07-09)

### 🍕 Features

* `YoloChart` in repo pages ([#3658](#3658)) ([9462519](9462519))

### 🐛 Bug Fixes

* now app sidebar menu items have the same font weight ([#3690](#3690)) ([3647252](3647252))
* now the repository insights pages header looks good on smaller screens ([#3687](#3687)) ([8a47e17](8a47e17))
* use explore page with typescript topic as fallback instead of feed page ([#3691](#3691)) ([a50e34a](a50e34a))
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.

Feature: YOLO Coders Component Feature: Skunkworks - implement "Yolo Pushes" metric
7 participants