Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
StatusFileSize
new58.43 KB

This should do it.

Status: Needs review » Needs work

The last submitted patch, 2: 3093219.patch, failed testing. View results

sam152’s picture

dinesh18’s picture

StatusFileSize
new1.8 MB

I have looked into the two tickets mentioned and it looks good to me. But we still have some more deprecated methods which we need to remove.
Attached is the screenshot after applying the patches mentioned in #4.
Agree with @Sam152 smaller issues would be easier to commit.

berdir’s picture

> Agree with @Sam152 smaller issues would be easier to commit.

Looking at the parent, one issue per module/component seems to be better standard. Some are smaller but several are just as big or bigger. I think these removal only issues are pretty easy to review, you just need one check per file/function to make sure it really should be removed.

longwave’s picture

Status: Needs work » Needs review
StatusFileSize
new46.68 KB

Rerolled and included remaining deprecations.

sam152’s picture

Status: Needs review » Reviewed & tested by the community

If large issues are being committed, I'm fine with it here too.

  • catch committed 6c69385 on 9.0.x
    Issue #3093219 by amateescu, longwave, Dinesh18: Remove...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Just to say for deprecation removals, especially when the diffstat is like this 16 files changed, 4 insertions, 1163 deletions., per-module seems the best way. Very little chance of conflicting with other deprecation removal patches (and most other patches shouldn't touch already-deprecated code either).

If we have actual bc-layer logic that's not simply a case of removing hunks of code it might be worth splitting out sometimes.

Committed 6c69385 and pushed to 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.