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

CXSPA/9197: Remove unused method in major release #19833

Closed

Conversation

anjana-bl
Copy link
Contributor

@anjana-bl anjana-bl requested a review from a team as a code owner January 7, 2025 08:50
@github-actions github-actions bot marked this pull request as draft January 7, 2025 08:51
@Platonn
Copy link
Contributor

Platonn commented Jan 9, 2025

a year ago we deprecated officially (via JSDocs which are visible to customers) the original method and told customers to use the V2 method instead. Thanks to this advice, some of them them might have moved to V2, to adapt to the future breaking changes in advance.

  1. So to be fair, now we should remove the original method which was deprecated and leave the replacement V2.
  2. Then we should also document somewhere in the migration docs draft (which are placed rather in the develop-next-major but not develop yet) that the original method was removed and the V2 should be used instead.

@anjana-bl anjana-bl marked this pull request as ready for review January 9, 2025 09:04

//TODO: CXINT-2896: Rename this method to `getOrderDetails` in next major release
getOrderDetailsV2(code: string): Observable<Order | undefined> {
getOrderDetails(code: string): Observable<Order | undefined> {
Copy link
Contributor

Choose a reason for hiding this comment

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

major: Since the getOrderDetails method was deprecated with the information to use getOrderDetailsV2 instead, we should keep the one with V2 suffix to be in line with the deprecation message (and to be fair to our customers).

@pawelfras
Copy link
Contributor

In addition to what Kris said, let's change target branch to develop-next-major and together with this PR, let's add migration information under 2211_ng18, similar to what we can find in 2211_19/2211-typescript-manual.doc.md

Copy link

cypress bot commented Jan 9, 2025

spartacus    Run #46589

Run Properties:  status check passed Passed #46589  •  git commit aaebb246ed ℹ️: Merge 586692f29a7754381046a0eaf3dcb53621965d51 into 89151c12a4d7c5b6240ee5abcb40...
Project spartacus
Branch Review CXSPA-9197-refactoring-myaccountv2
Run status status check passed Passed #46589
Run duration 11m 36s
Commit git commit aaebb246ed ℹ️: Merge 586692f29a7754381046a0eaf3dcb53621965d51 into 89151c12a4d7c5b6240ee5abcb40...
Committer anjana-bl
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 125
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@anjana-bl anjana-bl changed the base branch from develop to develop-next-major January 9, 2025 09:29
@anjana-bl anjana-bl requested review from a team as code owners January 9, 2025 09:29
Zeyber and others added 17 commits January 9, 2025 15:00
…CXSPA-9067)" (#19749)

Co-authored-by: Radhep Sabapathipillai <34665674+RadhepS@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
… from a category (#19576)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ner is visible and ensure clear visible focus for accessibility (#19728)
…BoxComponent (#19680)

Co-authored-by: Giancarlo Cordero Ortiz <46171897+giancorderoortiz@users.noreply.github.com>
Co-authored-by: Christoph Hinssen <33626130+ChristophHi@users.noreply.github.com>
…button (#19731)

Co-authored-by: Christoph Hinssen <33626130+ChristophHi@users.noreply.github.com>
uroslates and others added 24 commits January 9, 2025 15:05
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Giancarlo Cordero Ortiz <46171897+giancorderoortiz@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Giancarlo Cordero Ortiz <46171897+giancorderoortiz@users.noreply.github.com>
…19662)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Giancarlo Cordero Ortiz <46171897+giancorderoortiz@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Giancarlo Cordero Ortiz <46171897+giancorderoortiz@users.noreply.github.com>
@anjana-bl anjana-bl force-pushed the CXSPA-9197-refactoring-myaccountv2 branch from 586692f to b845a7e Compare January 9, 2025 09:39
@anjana-bl anjana-bl requested a review from a team as a code owner January 9, 2025 09:39
@github-actions github-actions bot marked this pull request as draft January 9, 2025 09:39
@anjana-bl anjana-bl closed this Jan 9, 2025
@anjana-bl anjana-bl deleted the CXSPA-9197-refactoring-myaccountv2 branch January 9, 2025 10:06
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.

None yet