LSF effect description for mode lms_cube

[oczoske]

Implementing LSF effect (AstarVienna/ScopeSim#670) for METIS lms_cube

[teutoburg]

This is now a feature branch based on another feature branch. Which is perfectly fine, but has to be treated with some care. I created some diagrams below to illustrate the situation (commit IDs and number of commits is simplified for the example).

Current situation

%%{init: {'gitGraph': {'mainBranchName': 'dev_master'}} }%%
gitGraph
   commit
   branch fh/simple-ifu
   commit
   commit
   commit
   branch oc/lsf_effect
   commit
   commit
   checkout dev_master
   merge fh/simple-ifu id: "merge 1"
   merge oc/lsf_effect id: "merge 2"

Loading

This is how it would look like if we first merged #233 and then this (the other way around wouldn't make sense). The issue here is that this PR here will still show the commits from fh/simple-ifu even after that branch is merged into dev_master. That seems a bit "unclean" to me.

Alternative

%%{init: {'gitGraph': {'mainBranchName': 'dev_master'}} }%%
gitGraph
   commit
   branch fh/simple-ifu
   commit
   commit
   commit
   branch oc/lsf_effect
   commit
   commit
   checkout fh/simple-ifu
   merge oc/lsf_effect id: "merge 1"
   checkout dev_master
   merge fh/simple-ifu id: "merge 2"

Loading

The other option, which I'd much prefer, is to first merge oc/lsf_effect into its parent fh/simple-ifu. Then that branch would receive the commits from here, and could be merged into dev_master at once. The PR here should then only show the commits done after branching off of fh/simple-ifu in the diff, which seems much more representative of what actually happened.

How to?

Should be as simple as changing the base for this PR to fh/simple-ifu instead of the default dev_master. I haven't done that many times, but I'm fairly sure it'll work. If not, we can always change it back, that doesn't destroy anything.

Alternative alternatives

Merging dev_master back into oc/lsf_effect after merging fh/simple-ifu

While that should also avoid the duplication of commtis, it seems like some unnecessary criss-cross merging and also I'm not entirely sure how the commits would be resolved. Better avoid.

Rebasing

Would certainly work, but involves force pushing, which is not so great if the PRs already exist.

Just merging this PR

Would also work, i.e. closing #233 unmerged and then only merging this PR, meaning we pretend fh/simple-ifu never existed and all the commits were just done on oc/lsf_effect. But that seems like "rewriting history" and having the closed PR in there might be confusing in the future. Better also avoid.

[oczoske]

I had changed the base earlier today but didn't want to proceed with it. I ran a simulation with oc/lsf_effect earlier today (against scopesim/main) and only had to change the needs_scopesim versions to include v. I'd say, go ahead with "Alternative". If anything should go awry it shouldn't be difficult to figure it out and fix it.

[teutoburg]

Changed the base as described in previous comment. To my relief, it now actually looks as I expected. Go ahead and merge 👍