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

Remove placeholder text #22274

Merged
merged 3 commits into from
Sep 20, 2024
Merged

Remove placeholder text #22274

merged 3 commits into from
Sep 20, 2024

Conversation

noahtalerman
Copy link
Member

@noahtalerman noahtalerman commented Sep 20, 2024

Issue

Cerra #22278

Description

  • Issue: It's unclear, at a glance, when I'm enforcing updates and when I'm not
  • Solution: Remove placeholder text

Screenrecording of fix

https://www.loom.com/share/774b1f8180054ed69a0c6bdd53897acf

- It's unclear, at a glance, when I'm enforcing updates and when I'm not
iansltx
iansltx previously approved these changes Sep 20, 2024
iansltx
iansltx previously approved these changes Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.20%. Comparing base (8d664bd) to head (b062299).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #22274   +/-   ##
=======================================
  Coverage   65.20%   65.20%           
=======================================
  Files        1496     1496           
  Lines      117239   117239           
  Branches     3512     3571   +59     
=======================================
  Hits        76447    76447           
+ Misses      33656    33655    -1     
- Partials     7136     7137    +1     
Flag Coverage Δ
frontend 52.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@noahtalerman
Copy link
Member Author

Thanks @iansltx! I didn't get a chance to QA this myself. Did you give it a quick check?

@sharon-fdm, assuming Ian helped QA, is it ok for me to merge this now? I'm not sure where we stand w/ 4.57 release.

Copy link
Member

@RachelElysia RachelElysia left a comment

Choose a reason for hiding this comment

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

Remove unused code:

  const getMinimumVersionPlaceholder = (platform: ApplePlatform) => {
    switch (platform) {
      case "darwin":
        return "13.0.1";
      case "ios":
      case "ipados":
        return "17.5.1";
      default:
        return "";
    }
  };

@sharon-fdm
Copy link
Collaborator

sharon-fdm commented Sep 20, 2024

@RachelElysia, can you please remind me which ticket this is for?
If this is not an unreleased bug, we can, of course, merge to main but not cherry-pick to the RC Branch.

cc @noahtalerman

@RachelElysia
Copy link
Member

@sharon-fdm - I don't know if this has a ticket. It looks like @noahtalerman just saw a UI improvement (these placeholders are released) and wanted to snipe it real quick.

@sharon-fdm
Copy link
Collaborator

sharon-fdm commented Sep 20, 2024

Ahhhhh... sounds good.
Then, once approved, feel free to merge to the main branch.
@noahtalerman, at this point, we only add critical things to the release. We could do an exception if there is a good reason though.

@RachelElysia

@iansltx
Copy link
Member

iansltx commented Sep 20, 2024

IMO no exception here, as we can pick this up in 4.57.1, and 4.57.0 is frozen for smoke testing at this point.

@iansltx
Copy link
Member

iansltx commented Sep 20, 2024

@noahtalerman I can QA, but will need a ticket so this doesn't get lost, as I still have some docs to write.

@iansltx
Copy link
Member

iansltx commented Sep 20, 2024

(and also Rachel's review absolutely wins over mine, so there's a little extra work to do here)

@RachelElysia RachelElysia merged commit fc8b1d6 into main Sep 20, 2024
14 checks passed
@RachelElysia RachelElysia deleted the noahtalerman-patch-9 branch September 20, 2024 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants