fix: remove project.approvals.set_approvals() method

[JohnVillalovos]

The project.approvals.set_approvals() method used the
/projects/:id/approvers end point. That end point was removed from
GitLab in the 13.11 release, on 2-Apr-2021 in commit
27dc2f2fe81249bbdc25f7bd8fe799752aac05e6 via merge commit
e482597a8cf1bae8e27abd6774b684fb90491835. It was deprecated on
19-Aug-2019.

See merge request:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57473

[nejch]

Thanks @JohnVillalovos - was gonna get to this myself but you beat me to it 😅

I think we can now also remove/modify this part and potentially start testing approvals in functional tests:

python-gitlab/tests/functional/api/test_merge_requests.py

Lines 132 to 134 in dd04e8e

	project.approvals.set_approvers([1], [])
	approval = project.approvals.get()
	assert approval.approvers[0]["user"]["id"] == 1

However, our project-level approval managers/objects are missing some mixing potentially.
Otherwise I just had some notes on the difference no project/MR-level approvals.

[JohnVillalovos]

Thanks @JohnVillalovos - was gonna get to this myself but you beat me to it 😅

I think we can now also remove/modify this part and potentially start testing approvals in functional tests:

python-gitlab/tests/functional/api/test_merge_requests.py

Lines 132 to 134 in dd04e8e

	project.approvals.set_approvers([1], [])
	approval = project.approvals.get()
	assert approval.approvers[0]["user"]["id"] == 1

I agree we should get the tests working. But I just wanted to remove the non-working endpoint at this stage. Getting the tests working is a separate issue and MR in my mind.

[nejch]

Thanks @JohnVillalovos - was gonna get to this myself but you beat me to it sweat_smile
I think we can now also remove/modify this part and potentially start testing approvals in functional tests:

python-gitlab/tests/functional/api/test_merge_requests.py

Lines 132 to 134 in dd04e8e

	project.approvals.set_approvers([1], [])
	approval = project.approvals.get()
	assert approval.approvers[0]["user"]["id"] == 1

I agree we should get the tests working. But I just wanted to remove the non-working endpoint at this stage. Getting the tests working is a separate issue and MR in my mind.

Ok, makes sense. the xfail is a good indicator anyway that there's something wrong with the code and we should fix it at some point.

Could we maybe do a fix: remove dead code in .. or something like that so it shows up in the changelog? I wouldn't consider it a breaking change for something from 13.x but just if someone does still use it, they'll see immediately why it's gone in the release :)

[JohnVillalovos]

Could we maybe do a fix: remove dead code in .. or something like that so it shows up in the changelog? I wouldn't consider it a breaking change for something from 13.x but just if someone does still use it, they'll see immediately why it's gone in the release :)

Done! 🙂 Good idea!

[JohnVillalovos]

@nejch @lmilbaum I also put a fix for #2334 in this PR.

[lmilbaum]

@nejch @lmilbaum I also put a fix for #2334 in this PR.

Cool. Than, I can remove the astroid pinned version in #2320.