Merge stable to develop

[ReimarBauer]

Purpose of PR?:

This needs a merge no squash

[matrss]

$ git grep -n '202[23] by the'
mslib/mscolab/message_type.py:10:    :copyright: Copyright 2019-2023 by the MSS team, see AUTHORS.
mslib/msui/msui_mainwindow.py:16:    :copyright: Copyright 2016-2023 by the MSS team, see AUTHORS.
mslib/utils/migration/config_before_nine.py:13:    :copyright: Copyright 2016-2023 by the MSS team, see AUTHORS.
mslib/utils/migration/update_json_file_to_version_nine.py:13:    :copyright: Copyright 2016-2023 by the MSS team, see AUTHORS.
tests/_test_mscolab/test_server_auth_required.py:12:    :copyright: Copyright 2020-2023 by the MSS team, see AUTHORS.
tests/fixtures.py:11:    :copyright: Copyright 2023 by the MSS team, see AUTHORS.
tutorials/utils/picture.py:11:    :copyright: Copyright 2016-2023 by the MSS team, see AUTHORS.
tutorials/utils/platform_keys.py:10:    :copyright: Copyright 2021-2023 by the MSS team, see AUTHORS.

[ReimarBauer]

@matrss you do look at develop, or?

In develop we have at least picture.py created which is not in stable.

We need after the merge a rest cleanup on develop.

[matrss]

I checked in this PR. Since this PR applies a "update our copyright string to 2024" commit I expect the result to no longer contain copyright strings that need to be fixed, because this PR is intended to fix them.

[ReimarBauer]

This PR is a merge from stable to develop. It can't change what it is new in develop.

[matrss]

This PR is a merge of the changes that were made on stable, to develop. I think this PR must consider the intention of these changes and adapt them to account for the differences between stable and develop, i.e. also fix the copyright strings of files that are only on develop. It is the same with the msidp entrypoint that was missing, which you have already fixed.

If these things aren't fixed then this PR would leave develop in a broken state.

[ReimarBauer]

Our releases are based on stable. Develop needs a way more changes to become the next stable.

Just file an issue for the missings in develop.

The entrypoint I have fixed because the modules were touched by the merge.
The others do not exists in stable.

This should become a merge only between both branches.

[matrss]

Our releases are based on stable. Develop needs a way more changes to become the next stable.

Yes, and? I don't see what this has to do with the issue.

Just file an issue for the missings in develop.

You requested a review from me, and I made a review noting the problems I see with this PR. I don't see a reason why these issues can't be fixed right here, right now, instead of messing up the develop branch. Fixing up the merged commits is part of the merge, in my opinion. If you intentionally want this to merge a broken set of changes that is fine, but please tell me before hand. In that case I will no longer review these PRs though, since I can't review them if I don't know what kind of breakage is intentional and what is not.

[joernu76]

Moin, I think we should discuss offline the intent of these merge requests from stable to develop.

The primary intent was to bring "hot fixes" from the stable branch back to the develop branch. So far we reviewed them mostly for "technical" problems, less for content.
If a numbered issue was addressed in stable and the fix would not address the same issue in develop, we have indeed a problem (as it will be automatically closed when brought to stable AFAIK).

I am not sure how to solve the issue at hand technically. As the stable branch does not contain the relevant files, it is not straightforward to add the fix to the PR. I would suggest to a) not fix such stuff as copyright notices in the stable branch going forward and b) to manually reopen the issue after the merge and fix it properly in a separate MR.

Nice catch, BTW.

[matrss]

If a numbered issue was addressed in stable and the fix would not address the same issue in develop, we have indeed a problem (as it will be automatically closed when brought to stable AFAIK).

What I've noted is exactly this kind of problem, the corresponding issue for the copyright notice update was closed after it went to stable, but it would not be fixed with this merge to develop.

I am not sure how to solve the issue at hand technically. As the stable branch does not contain the relevant files, it is not straightforward to add the fix to the PR.

Since the PR branch is based on develop and merges in the commits from stable, it does contain the files whose copyright wasn't updated. So a commit to fix them could be added, either as a separate commit after the merge or (I think) squashed into the merge commit.

---

I will retract my change request so this PR can be merged if you want to, but I still think this kind of workflow makes no sense. It merges changes from stable into develop without keeping their intention intact, which will lead to issues down the line (e.g. the fact that there were still copyright notices for 2022 that were missed in the update last year; or the missing msidp entrypoint that I just noticed because I prepared that PR on develop, rebased it onto stable because apparently it should go there and then caught this as a merge conflict going in the other direction, and made a mental note to look out for it whenever the stable->develop merge happens).

FWIW I have written down my thoughts on this a while ago already, here: #2206. Included is an outline of an alternative workflow that would avoid merges completely and resolves these kinds of merge conflicts on a change-by-change basis.

We can also discuss this in one of the meetings if you want to, I'll refrain from reviewing these PRs until then.

Disagreement on how to review merge PRs