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 new show notes endpoint FF #2527

Merged
merged 3 commits into from
Dec 11, 2024

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Dec 10, 2024

| 📘 Part of: #2509 |
|:---:|

Fixes #2528

Removes the FF for using the new show notes endpoint

To test

  1. Start the app
  2. Open a podcast
  3. Open an episode detail page
  4. Check if information shows correctly
  5. Open another podcast
  6. Tap on the more button
  7. Tap on Select Episodes
  8. Select one episode
  9. Tap on download
  10. Wait for download to finish
  11. Go offline
  12. Open the episode
  13. Check if details show correctly.

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@SergioEstevao SergioEstevao added the [Type] Tech Debt Applies to issues involving upgrades or refactoring to maintain or enhance the codebase. label Dec 10, 2024
@SergioEstevao SergioEstevao added this to the 7.80 milestone Dec 10, 2024
@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@SergioEstevao SergioEstevao marked this pull request as ready for review December 10, 2024 15:10
@SergioEstevao SergioEstevao requested a review from a team as a code owner December 10, 2024 15:10
@SergioEstevao SergioEstevao requested review from danielebogo and leandroalonso and removed request for a team December 10, 2024 15:10
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

It works as described.

We should also remove the condition from the Firebase configuration. Wdyt?

@SergioEstevao
Copy link
Contributor Author

We should also remove the condition from the Firebase configuration. Wdyt?

Sure thing, if Android is not using it I think we can remove it for sure

@SergioEstevao SergioEstevao merged commit a22de83 into trunk Dec 11, 2024
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the remove/ff_new_show_notes_endpoints branch December 11, 2024 14:06
@@ -164,8 +161,6 @@ public enum FeatureFlag: String, CaseIterable {
false
case .endOfYear:
false
case .newShowNotesEndpoint:
false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielebogo Just realized that we cannot remove the remote FF from Firebase, because of the above.
The default value is set to false so the older version where this was activated because of the remote FF will have the functionality disabled because the default value on the code is a false.
Am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Tech Debt Applies to issues involving upgrades or refactoring to maintain or enhance the codebase.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove newShowNotesEndpoint FF
3 participants