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

Dependency Updates - Other Batch #17559

Closed
3 tasks done
ParaskP7 opened this issue Nov 29, 2022 · 10 comments
Closed
3 tasks done

Dependency Updates - Other Batch #17559

ParaskP7 opened this issue Nov 29, 2022 · 10 comments

Comments

@ParaskP7
Copy link
Contributor

ParaskP7 commented Nov 29, 2022

Parent #17551

This issue is about updating all Other related dependencies for the whole project.

This Other batch contains the following 3 dependencies:

@peril-wordpress-mobile
Copy link

Fails
🚫

Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

1 similar comment
@peril-wordpress-mobile
Copy link

Fails
🚫

Please add a type label to this issue. e.g. '[Type] Enhancement'

Generated by 🚫 dangerJS

@ParaskP7 ParaskP7 mentioned this issue Nov 29, 2022
50 tasks
@ParaskP7
Copy link
Contributor Author

✅ The latest stable version of stethoVersion is 1.6.0 (see releases). As such, this item is now marked as complete.

❓ However, maybe we should take this opportunity to upgrade from Stetho to Flipper. 🤔

FYI: WCAndroid is already using Flipper instead.

@ParaskP7
Copy link
Contributor Author

✅ The latest stable version of androidDesugarVersion is 1.1.5 (see tags). As such, this item is now marked as complete.

❓ I am not quite sure if we still need this dependency, we should double-check that and move forward accordingly.

FYI: WCAndroid is not using such a dependency, or any coreLibraryDesugaring related dependency.

@ParaskP7
Copy link
Contributor Author

❓ I am not sure what's the latest stable dependency for wordPressLintVersion (see repo). Also, I am not quite sure whether we need these 3 custom Lint rules any more (see below). Maybe we don't and thus we can deprecated this repo as well. 🤔

Custom Lint Rules:

@oguzkocer
Copy link
Contributor

@ParaskP7 1.1.0 is the latest version we have for wordPressLintVersion in our S3 Maven, so I am fairly certain that it's the latest available version. We had issues adding Buildkite publishing to the repo, so even if we add/change anything, there is no easy way to publish them at the moment.

I am generally in favor of using commonly available lint rules, rather than custom ones because of the burden of maintaining them. Combining that with the fact that we don't have publishing implemented for the repo, I'd be in favor of dropping this dependency unless there is a significant reason to keep it.

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Dec 6, 2022

@ParaskP7 1.1.0 is the latest version we have for wordPressLintVersion in our S3 Maven, so I am fairly certain that it's the latest available version.

Thanks for the confirmation on that @oguzkocer ! 🙇

We had issues adding Buildkite publishing to the repo, so even if we add/change anything, there is no easy way to publish them at the moment.

Good to know, thanks for noting that! 🌟

I am generally in favor of using commonly available lint rules, rather than custom ones because of the burden of maintaining them.

Same here! 💯

Especially due to the fact that those Lint rules are old and might not even apply anymore to our project(s), we should at least consider checking them out, assert their values, try to replace them with commonly available Lint rules (if any), then remove this dependency (from this and all other client and lib repos) and stop maintaining this custom Lint repo altogether. 🤔

Combining that with the fact that we don't have publishing implemented for the repo, I'd be in favor of dropping this dependency unless there is a significant reason to keep it.

I agree! 💯

@oguzkocer
Copy link
Contributor

@ParaskP7 Since wordPressLintVersion = '1.1.0' is the latest version, I've marked that as completed and will close this issue. We can follow up on removing the dependency as a separate task when we can.

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Dec 9, 2022

@ParaskP7 Since wordPressLintVersion = '1.1.0' is the latest version, I've marked that as completed and will close this issue. We can follow up on removing the dependency as a separate task when we can.

I agree @oguzkocer , thank you! 🥇

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented Mar 2, 2023

✅ The latest stable version of androidDesugarVersion is 1.1.5 (see tags). As such, this item is now marked as complete.

I stand corrected on that because there actually are newer versions of this Android Desugar JDK library (see Docs and Google's Maven Repository.

❓ I am not quite sure if we still need this dependency, we should double-check that and move forward accordingly.

I stand corrected on that too, this Android Desugar JDK library is needed and was introduced in this PR to help with making date related operations during the Notifications project, which the java.time API would have made quite painful otherwise.

FYI: WCAndroid is not using such a dependency, or any coreLibraryDesugaring related dependency.

Once more, I stand correct here too since WCAndroid is about to use this Android Desugar JDK library as well (see PR), for the very same reasons the WPAndroid team did.


However, updating this library is blocked by the AGP 7.3.0 (for 1.2.2) and AGP 7.4.0 (for 2.0.2). Thus, this update isn't possible just yet. For more info on that see this WCAndroid related code review comment here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants