-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Migrate deprecated APIs and suppress compiler warnings (7) #7553
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
base: master
Are you sure you want to change the base?
Conversation
The recommendation is to switch to the UniformRandomProvider. This would however add a new dependency that's only used in the unit tests. [1] https://issues.apache.org/jira/browse/LANG-1604
…tAsLong The buyerSecurityDepositAsLong field was superseded by the buyerSecurityDepositAsPercent field.
It's too risky to refactor the code because it's used in the account age verification logic.
WalkthroughThis pull request applies deprecation warning suppression annotations across numerous payment account payload classes, core services, and utilities. Additionally, it removes the deprecated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (11)
core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (1)
80-88: Scoped deprecation suppression looks appropriateUsing
@SuppressWarnings("deprecation")at thefromProtomethod level keeps the suppression narrow and avoids changing runtime behavior, which is exactly what you want for legacy proto access. No further changes needed here.If you plan a future cleanup pass, consider adding a short comment pointing to the deprecation source (e.g., which proto field or accessor is deprecated) to make it easier to remove this suppression later.
core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1)
74-84: Scoped deprecation suppression looks good; consider renaming local var for clarityAdding
@SuppressWarnings("deprecation")at the method level is appropriate here and keeps the suppression narrowly scoped to this legacy-fromProto path. As a small readability improvement, you might also renamepaytmAccountPayloadPBso it matches the Bizum context:- protobuf.BizumAccountPayload paytmAccountPayloadPB = countryBasedPaymentAccountPayload.getBizumAccountPayload(); + protobuf.BizumAccountPayload bizumAccountPayloadPB = countryBasedPaymentAccountPayload.getBizumAccountPayload(); @@ - paytmAccountPayloadPB.getMobileNr(), + bizumAccountPayloadPB.getMobileNr(),desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java (1)
93-111: Consider usingbuffer.stream()to eliminate the unchecked cast.Rather than suppressing the warning, you can refactor line 104 to use the Collection's built-in
stream()method, which avoids the unchecked cast entirely and is more efficient.Apply this diff:
- @SuppressWarnings("unchecked") @Override public boolean tryAdvance(Consumer<? super Stream<T>> action) { if (windowSize < 1) { return false; } while (sourceIterator.hasNext()) { buffer.add(sourceIterator.next()); if (buffer.size() == windowSize) { - action.accept(Arrays.stream((T[]) buffer.toArray(new Object[0]))); + action.accept(buffer.stream()); buffer.poll(); return true; } } return false; }desktop/src/test/java/bisq/desktop/ComponentsDemo.java (1)
52-54: Unchecked suppression scope could be narrowerThe suppression is fine for this demo, but it currently covers the entire
startmethod. If you touch this again, consider typingObservableList<String>(and dropping the suppression) or moving@SuppressWarnings("unchecked")to just the affected local variable so the scope stays minimal.core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (1)
105-115: Narrow deprecation suppression on builder looks goodPlacing
@SuppressWarnings("deprecation")on the localbuilderkeeps the warning tightly scoped while preserving the existing proto builder usage for compatibility. Consider adding a short comment later explaining why the deprecated builder is still required, but not strictly necessary here.core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (1)
89-100: Deprecation suppression is consistent; consider tightening return type laterThe added
@SuppressWarnings("deprecation")onfromProtois consistent with other payload classes and keeps warnings localized to proto construction. If you ever touch this again, you might consider returningMoneyGramAccountPayloadinstead ofPaymentAccountPayloadfor a more precise API, but that's not required for this warning-cleanup PR.desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java (1)
126-141: Suppression is fine; consider a generics-friendly follow-upThe
@SuppressWarnings({"rawtypes", "unchecked"})aroundloadViewis appropriately scoped and avoids leaking raw-type noise elsewhere. Longer term, you could replace the rawTuple2cast with a generics-safe signature (e.g.,Tuple2<?, ?>and a typedfillFromTradeData) to drop the need for these suppressions, but that's an optional cleanup beyond this PR.core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (1)
88-100: Scoped deprecation suppression onfromProtolooks fineUsing
@SuppressWarnings("deprecation")at method scope is appropriate here since this factory must keep calling deprecated proto fields for backward compatibility. Consider a brief comment indicating it relies on legacy proto fields so future cleanup is easier.core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (1)
67-74: Deprecation suppression onfromProtois consistent with other payloadsThis keeps the legacy proto deserialization path without altering behavior and matches the pattern elsewhere in the PR. If you expect to eventually drop the deprecated proto fields, a short comment here noting that this method is legacy-only could help future cleanup.
core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (1)
142-165: Legacy Swift deserialization suppression is reasonableMarking
fromProtowith@SuppressWarnings("deprecation")is appropriate given it depends on deprecated Swift-related proto fields andmax_trade_period. No behavioral change introduced. You might add a comment that this path exists purely for backward compatibility to signal future removal.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java (1)
57-181: Deprecation suppressions are correctly limited to the legacy conversion pathAll three
@SuppressWarnings("deprecation")annotations are on code that necessarily touches deprecatedTradeStatistics2/ proto fields to perform one-way conversion toTradeStatistics3. That’s an appropriate place to mute warnings without changing behavior. You may want to add a class‑ or method‑level comment noting this is a temporary migration layer so it can be removed onceTradeStatistics2data is fully gone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (75)
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java(2 hunks)core/src/main/java/bisq/core/offer/OpenOfferManager.java(1 hunks)core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java(1 hunks)core/src/main/java/bisq/core/proto/CoreProtoResolver.java(2 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java(3 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java(1 hunks)core/src/main/java/bisq/core/user/PreferencesPayload.java(0 hunks)desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java(1 hunks)desktop/src/main/java/bisq/desktop/components/MenuItem.java(1 hunks)desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java(1 hunks)desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java(1 hunks)desktop/src/test/java/bisq/desktop/ComponentsDemo.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java(1 hunks)proto/src/main/proto/pb.proto(1 hunks)
💤 Files with no reviewable changes (1)
- core/src/main/java/bisq/core/user/PreferencesPayload.java
🔇 Additional comments (60)
core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (1)
89-106: Clarify the deprecation suppression strategy across payload classes.The
@SuppressWarnings("deprecation")annotation correctly suppresses the compiler warning for this method. However, the PR objectives mention removing deprecated fields (e.g.,buyerSecurityDepositAsLongfromPreferencesPayloadwith protobuf updates), while this file opts to suppress the warning instead.Questions for clarity:
- Is the deprecation originating from the protobuf definitions themselves (e.g., deprecated proto fields), or from Java-level APIs?
- Why does this file suppress the warning rather than addressing the root cause via protobuf definition updates, as appears to be done elsewhere in the PR?
- Is this suppression consistently applied across all similar
fromProtomethods in payment payload classes?If suppression is the intended approach for protobuf-level deprecations (as opposed to Java API deprecations), consider adding a brief inline comment explaining the rationale—particularly if other files in this PR take a different approach.
core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2)
231-282: Scoped deprecation suppression ontoProtoMessageis reasonableAdding
@SuppressWarnings("deprecation")at the method level keeps the deprecation noise localized to this backward‑compat proto serialization logic without affecting the rest of the class. No behavioral changes introduced; this fits the PR goal of cleaning build warnings.
284-337: Scoped deprecation suppression onfromProtois consistent and safeApplying
@SuppressWarnings("deprecation")onfromProtomirrorstoProtoMessageand keeps compatibility reads from deprecated proto fields while avoiding global suppression. The method behavior is unchanged, so this is a low‑risk, build‑noise reduction.desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java (1)
242-244: Method-level deprecation suppression looks goodAdding
@SuppressWarnings("deprecation")onaddContent()is a reasonable, narrowly scoped way to quiet the deprecation warnings here without affecting other code. No behavioral impact, and the annotation ordering with@Overrideis fine.p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java (1)
47-51: Targeted unchecked-warning suppression in test fake looks goodAdding
@SuppressWarnings("unchecked")on this constructor is appropriate given the genericPersistenceManagermock in thesuper(...)call, and keeping the suppression scoped to the constructor (instead of the whole class) is a good practice. No behavior change; aligns with the PR’s goal of cleaning up test-time warnings.p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (1)
32-35: Consistent suppression of unchecked warning on helper test serviceUsing
@SuppressWarnings("unchecked")on this constructor is reasonable for a test-only fake that wires generic services, and it keeps the suppression narrowly scoped. It also stays consistent withMapStoreServiceFake, matching the PR’s intent to remove noisy test warnings without altering behavior.desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java (1)
612-613: Document the suppressed deprecation warning and verify future migration plan.The
@SuppressWarnings("deprecation")annotation hides compiler warnings without addressing the underlying deprecated API usage. Please add an inline comment above this annotation identifying:
- Which specific API(s) are deprecated (e.g., constructor parameter types, method calls)
- Why the deprecated API cannot be replaced currently
- Whether a migration plan exists for future refactoring
This documentation will help maintainers understand the suppression rationale and track technical debt.
core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (1)
73-80: Verify the deprecated API and document the suppression rationale.The
@SuppressWarnings("deprecation")annotation suppresses warnings without indicating which specific API is deprecated or whether there's a migration plan. Consider adding a TODO comment explaining the deprecation context and target version for removal, following protobuf migration best practices.core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (1)
74-84: Deprecation suppression on fromProto is appropriateAnnotating
fromPrototo suppress proto deprecation warnings is consistent with the other payloads and keeps the legacy deserialization path without changing behavior.core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (1)
82-94: Consistent deprecation suppression on fromProtoThis matches the pattern used in other payloads: deprecation noise is confined to the proto deserialization method, with no impact on runtime behavior.
core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java (1)
78-89: fromProto deprecation suppression is aligned with other payloadsAdding
@SuppressWarnings("deprecation")here keeps proto deprecation warnings contained to this legacy deserialization path without altering logic.core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (1)
94-109: Deprecation suppression on DomesticWireTransfer fromProto is fineThe suppression cleanly encapsulates deprecated proto usage inside the static
fromProtofactory, maintaining existing null/empty handling semantics.core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java (1)
72-79: PromptPay fromProto deprecation suppression is appropriateThis keeps deprecated proto access isolated in the
fromProtoconstructor path and is consistent with the other payment payloads in this PR.core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (1)
72-80: Monese fromProto deprecation suppression is consistent and safeThe added
@SuppressWarnings("deprecation")onfromProtoneatly silences proto deprecation warnings without touching the existing mapping logic.core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java (1)
74-84: LGTM!The
@SuppressWarnings("deprecation")annotation is correctly placed on thefromProtomethod. This is a standard approach for maintaining backward compatibility with deprecated protobuf fields while keeping the build warning-free.core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (1)
82-91: LGTM!The deprecation suppression is correctly applied to the
fromProtomethod, consistent with the pattern used across other payment payload classes in this PR.core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java (1)
94-107: LGTM!The deprecation suppression annotation is correctly placed on the
fromProtomethod, following the same pattern as other payment payload classes.apitest/src/test/java/bisq/apitest/scenario/TradeTest.java (2)
80-87: Acceptable targeted suppression.Placing
@SuppressWarnings("deprecation")on the local variable declaration is valid Java and provides more precise scoping than a method-level annotation. This correctly suppresses the warning for the deprecatedTakeBuyBSQOfferTestconstructor.
103-110: Consistent with the pattern above.Same targeted approach applied to
TakeSellBSQOfferTestinstantiation.core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1)
70-77: LGTM!The deprecation suppression is correctly applied, consistent with the project-wide pattern for
fromProtomethods in payment payload classes.core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (1)
72-79: LGTM!The
@SuppressWarnings("deprecation")annotation is appropriately scoped to just thefromProtomethod. This is the correct approach for suppressing warnings from deprecated protobuf accessors while keeping the suppression as narrow as possible.core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (1)
102-121: LGTM!The deprecation suppression is correctly scoped at the method level, consistent with the pattern used across other payment payload classes in this PR.
desktop/src/main/java/bisq/desktop/components/MenuItem.java (1)
145-148: LGTM!Excellent use of local variable-scoped
@SuppressWarnings("unchecked"). This is the narrowest possible suppression scope for this unavoidable generic array creation warning.core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java (1)
83-92: LGTM!The suppression annotation follows the same consistent pattern as other payment payload classes in this PR.
desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java (1)
200-237: LGTM!The local variable-scoped
@SuppressWarnings("deprecation")is the ideal approach here, as it limits the suppression to just the deprecatedOfferPayloadconstructor call rather than the entire method.core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (1)
80-92: Method-level deprecation suppression is appropriate hereLimiting
@SuppressWarnings("deprecation")tofromProtokeeps the scope tight and avoids masking deprecations elsewhere. No behavioral change; mapping from proto to fields remains intact.core/src/main/java/bisq/core/offer/OpenOfferManager.java (1)
959-1074: Scoped deprecation suppression on migration helper looks goodApplying
@SuppressWarnings("deprecation")directly tomaybeUpdatePersistedOfferskeeps deprecation noise confined to this legacy-migration path without altering its existing upgrade logic.p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java (1)
67-85: Targeted deprecation suppression on test is appropriateAdding
@SuppressWarnings("deprecation")ontoProtoBufconfines the deprecation noise to this single test case while preserving coverage of the deprecated path. No behavioral changes.core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (1)
95-104: Consistent deprecation suppression for Revolut fromProtoThe added
@SuppressWarnings("deprecation")cleanly scopes deprecation handling tofromProtoand matches the pattern used in other payment payloads in this PR, with no impact on how Revolut accounts are deserialized.core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (1)
94-110: Deprecation suppression on ACH fromProto matches project patternApplying
@SuppressWarnings("deprecation")tofromProtois in line with other payment payloads and keeps deprecated proto usage warnings contained to this deserialization path. The ACH field mapping itself is unchanged.core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (1)
71-78: Localized deprecation suppression on HalCash fromProto is acceptableThe
@SuppressWarnings("deprecation")annotation onfromProtois narrowly scoped and aligns with the broader effort in this PR to quiet deprecation warnings on proto-based constructors without changing behavior.core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (1)
89-106: Verify the deprecated API and migration plan.The
@SuppressWarnings("deprecation")annotation hides compiler warnings without addressing the underlying deprecated API usage. While suppression may be appropriate as an interim measure, best practice is to migrate away from deprecated APIs.Please confirm:
- What specific deprecated API/method/field is being accessed in this
fromProtomethod?- Is there a migration plan or tracking issue to replace the deprecated usage?
- Is this suppression temporary or permanent?
desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java (1)
60-100: Clarify or remove the unnecessary deprecation warning suppression.Based on standard Java and JavaFX APIs, neither
Method.getDefaultValue()(the reflection call used here) nor the coreFXMLLoaderfunctionality is deprecated. The deprecatedFXMLLoadermethods (loadType()) are not used in this code. The@SuppressWarnings({"deprecation"})suppression appears unnecessary—either remove it or document which specific deprecated API or internal Bisq method it targets. If the deprecation is in the hiddengetDefaultValue()orloadFromFxml()methods, document that dependency.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java (1)
196-244: Verify TradeStatistics2 deprecation status and migration timeline directly in the codebase.The
@SuppressWarnings("deprecation")annotation at line 196 indicates a deprecation warning is being suppressed, but the review comment's verification requests cannot be confirmed without inspecting theTradeStatistics2class definition and related documentation in the repository. Please verify:
- Whether
TradeStatistics2is marked with@Deprecatedannotation- Any deprecation notes, TODOs, or comments documenting the migration timeline
- If there are plans or issues tracking removal of the compatibility check (lines 223-231)
Once verified, add a comment in the method explaining the deprecation status and expected timeline for removing this compatibility layer if applicable.
core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java (1)
77-85: Method-level deprecation suppression onfromProtolooks goodPlacing
@SuppressWarnings("deprecation")directly onfromProto(protobuf.PaymentAccountPayload)is scoped and consistent with the rest of the payloads, and it doesn’t change runtime behavior.core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (1)
80-92: Scoped deprecation suppression onfromProtois appropriate
@SuppressWarnings("deprecation")onfromProtokeeps the existing proto-based factory intact while silencing legacy field warnings, with no impact on behavior.core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (1)
81-90: Deprecation suppression onfromProtomatches the intended patternAdding
@SuppressWarnings("deprecation")tofromProtocleanly hides deprecated proto access warnings without altering serialization/deserialization behavior.core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (1)
75-86:@SuppressWarnings("deprecation")onfromProtois correctly appliedThe annotation is method-scoped, keeps the API unchanged, and aligns with the broader suppression strategy for proto-based factories in this PR.
core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java (1)
86-96: Interac e-TransferfromProtodeprecation suppression is fineMethod-level
@SuppressWarnings("deprecation")here is a minimal, behavior-neutral way to silence deprecated proto access while keeping deserialization logic unchanged.core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (1)
74-84: StrikefromProtodeprecation suppression is consistent and safeThe added
@SuppressWarnings("deprecation")is correctly scoped tofromProtoand does not affect the construction logic or public API.core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (1)
74-84: PaytmfromProtodeprecation suppression aligns with the PR’s goalAnnotating
fromProtowith@SuppressWarnings("deprecation")keeps deprecation noise out of the build while leaving the Paytm payload’s proto mapping intact.core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1)
75-82: MoneyBeamfromProtodeprecation suppression is correctly scopedThe new
@SuppressWarnings("deprecation")onfromProtois minimal and consistent with the rest of the codebase changes; it doesn’t alter the factory behavior.core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (1)
75-83: SwishfromProtodeprecation suppression matches the overall migration strategyThis is consistent with other payment payloads that must still read deprecated proto fields. No behavior change, just a cleaner build. Nothing else to change here.
core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (1)
73-80: AdvancedCashfromProtosuppression is appropriate for legacy proto usageThe deprecation suppression is correctly placed on the factory that still needs to read deprecated proto fields, with no behavior change. This keeps warnings localized to the compatibility boundary.
core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (1)
97-129: SEPA proto (de-)serialization deprecation suppressions are justifiedBoth
toProtoMessageandfromProtolegitimately depend on deprecated proto fields (e.g., the legacymax_trade_period) for backward‑compatibility and contract hash stability, so muting deprecation warnings at method level is appropriate. The existing field comment already hints at this, so no further change seems necessary.proto/src/main/proto/pb.proto (1)
1922-1994: Reserving field 30 forPreferencesPayloadis the correct protobuf patternReplacing the deprecated
buyer_security_deposit_as_longwithreserved 30;prevents accidental field tag reuse and maintains backward compatibility—older serialized data remains parseable. Ensure all code references to the removed field and its accessors have been cleaned up across the codebase, including generated code and any manual usages.core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (1)
101-115: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (2)
75-84: Deprecation suppression on serialization method.Unlike most other payload classes in this PR, this file adds suppression to
toProtoMessage()as well, suggesting deprecated fields are being written. The
86-95: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (1)
77-85: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java (1)
67-74: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java (1)
77-85: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java (1)
76-86: Consistent deprecation suppression applied.core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1)
67-74: Consistent deprecation suppression applied.core/src/main/java/bisq/core/proto/CoreProtoResolver.java (1)
101-101: Clarify which deprecated proto APIs are in scope and provide a migration timeline if applicable.The @SuppressWarnings("deprecation") annotations are appropriately scoped at the method level. However, without knowing which specific deprecated protobuf APIs are accessed at lines 101 and 261, and whether a migration plan exists, it's unclear if these suppressions are a temporary measure or part of a longer-term refactoring strategy. Consider documenting the deprecated APIs involved and any planned remediation.
core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (1)
74-74: LGTM - Systematic deprecation suppression applied.The method-scoped suppression annotation is appropriate for handling deprecated protobuf API usage during deserialization.
core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java (1)
114-114: LGTM - Consistent with deprecation suppression pattern.The annotation appropriately suppresses deprecation warnings for protobuf deserialization.
core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (1)
67-67: LGTM - Deprecation suppression correctly applied.The annotation follows the established pattern for handling deprecated protobuf APIs in deserialization methods.
core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (1)
70-70: LGTM - Appropriate deprecation handling.The suppression annotation is correctly scoped to the protobuf deserialization method.
core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (1)
72-72: LGTM - Deprecation suppression properly scoped.The annotation appropriately handles deprecated protobuf API usage during deserialization, consistent with the PR's systematic approach.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.