Skip to content

Address feedback from 2025-03-27 meeting #1353

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

Merged
merged 3 commits into from
Apr 1, 2025

Conversation

jcscottiii
Copy link
Collaborator

@jcscottiii jcscottiii commented Apr 1, 2025

This PR addresses the feedback from the 2025-03-27 meeting. It includes some breaking changes, but these affect features that were not yet public, specifically relating to the chromium usage statistics. These changes are considered acceptable because this PR also removes the associated feature flag, effectively making the feature available in its public form.


Changes:

Change 1: Extend the cache TTL

  • For stats page to 24 hrs for those routes

Change 2: Remove feature flags

  • Remove feature flag for usage chart
    • Corresponding Playwright tests and snapshots that tested this previously hidden feature were also removed, allowing standard snapshot tests to cover it moving forward.
  • Remove feature flag for missing in one chart
    • Corresponding Playwright tests and snapshots for this previously hidden feature were also removed, allowing standard snapshot tests to cover it moving forward.

Change 3: Change Chromium Usage to Chrome Usage

  • Display text on the website was changed from "Chromium Usage" to "Chrome Usage".
  • Changed the sort enum from chromium_usage to chrome_usage
    • Note (Breaking Change): This is technically an API breaking change. Users directly passing the old chromium_usage enum value via the API will now receive default sorting instead of an error. Direct usage of this enum is believed to be minimal or non-existent.
  • Rename the the route and other openapi aspects from chromium to chrome
    • Note (Breaking Change): This is an API breaking change. Users directly accessing the old route will receive a 404 error. But I think it's okay since it was not a public feature
  • Rename the chromium_usage field in the main features body to chrome_usage.
    • Note: This field has always been optional, so API consumers should already handle its potential absence gracefully.

Changes:

Change 1: Extend the cache TTL
- For stats page to 24 hrs for those routes

Change 2: Remove feature flags
- Remove feature flag for usage chart
- Remove feature flag for missing in one chart

Change 3: Change `Chromium Usage` to `Chrome Usage`
- Changed the text on the website from Chromium Usage to Chrome Usage
- Changed the sort enum from chromium_usage to chrome_usage
  - This is technically a breaking change. But users still passing chromium_usage will just get the default sorting (instead of an error)
  - Given this feature has not been made public yet, I think it is okay to change it now.
- Rename the the route and other openapi aspects from chromium to chrome
@jcscottiii jcscottiii requested review from jrobbins and KyleJu April 1, 2025 19:23
@jrobbins
Copy link
Collaborator

jrobbins commented Apr 1, 2025

It was always optional so others should make sure they did

Something was left off at the end of your PR description.

@jcscottiii
Copy link
Collaborator Author

It was always optional so others should make sure they did

Something was left off at the end of your PR description.

@jrobbins updated. Thanks!

@jcscottiii jcscottiii added this pull request to the merge queue Apr 1, 2025
Merged via the queue into main with commit aab5614 Apr 1, 2025
6 checks passed
@jcscottiii jcscottiii deleted the jcscottiii/feedback-2025-03-27 branch April 1, 2025 20:27
@jrobbins jrobbins mentioned this pull request Apr 2, 2025
Copy link

@CSingendonk CSingendonk left a comment

Choose a reason for hiding this comment

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

Hello?

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.

3 participants