Thesis of @anonym-HPI

[anonym-HPI]

Replaced by #523

TODO:

• dataStructure to find (more) efficiently elements around a specific element (patient, personnel or material)
• incremental updates instead of calculating every treatment of every patient every time from scratch
• change how many patients personnel and material can treat
• notarzt aura (TODO: make aura uninteractable and best would be behind patients) - Aura got removed instead
• fix tests / write tests for spatialTree (?)
• write migrations

benchmarking will be done outside of this PR, in this branch:
https://github.com/anonym-HPI/digital-fuesim-manv/tree/experimental/bachelors-thesis-marvin-benchmarking

[Dassderdie]

@anonym-HPI FYI:

You could also try to add the quadtree to the state by serializing and deserializing it:

// retrieving the quadtree from the state:
const quadtree = quadtree(state.quadtreeData);
// updating the quadtree in the state
draftState.quadtreeData = quadtree.data();

You would need to make sure that:

• the data in the quadtree is not mutated by it (I'm not 100% sure about this)
• These operations are not too expensive (benchmarks)
• data is a JSON object (no functions or relying on references)

[Dassderdie]

My second iteration over the changes here. I'll apply some of the straight forward stuff today.
The next big challenge will probably be the migrations...

[Dassderdie]

From my side, this is now (finally!) ready to be merged.

[ClFeSc]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

Has all been addressed and a second review could be done on the new branch.

[Dassderdie]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

Probably the best strategy. -> This branch gets replaced by https://github.com/hpi-sam/digital-fuesim-manv/tree/feature/improve-treatment-system.

[anonym-HPI]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

This could be, because not enough reviews approved it, maybe?

[Dassderdie]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

This could be, because not enough reviews approved it, maybe?

Actions always ran independent from the number of approving reviews.
Reviewers should also be able to see the results of the pipeline before approving an pull request.
Creating a new branch in this repo and merging in there also fixed this problem...

[anonym-HPI]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

This could be, because not enough reviews approved it, maybe?

Actions always ran independent from the number of approving reviews.
Reviewers should also be able to see the results of the pipeline before approving an pull request.
Creating a new branch in this repo and merging in there also fixed this problem...

I don't know what the default is in Github, but this mechanism is available (or something like that). Think about 10000 forks creating PRs and costing the repo owner money or taking away every free pipeline minute. Don't know if it could run on the forked pipeline or if it does. I saw this on another bigger repo (but then you have problems when secrets are used, as they will not be transfered to the fork), without some kind of approval (maybe some other kind of approval) or so, the pipeline didn't start.

But I get that a reviewer would like to review a PR that checks all automatic checks first.

[ClFeSc]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

This could be, because not enough reviews approved it, maybe?

Actions always ran independent from the number of approving reviews.
Reviewers should also be able to see the results of the pipeline before approving an pull request.
Creating a new branch in this repo and merging in there also fixed this problem...

I don't know what the default is in Github, but this mechanism is available. Think about 10000 forks creating PRs and costing the repo owner money or taking away every free pipeline minute. Don't know if it could run on the forked pipeline or if it does. I saw this on another bigger repo (but then you have problems when secrets are used, as they will not be transfered to the fork), without some kind of approval (maybe some other kind of approval) or so, the pipeline didn't start.

You can specifically configure that. For this repo, an approval is required for Actions of PRs by first-time contributers (or something similar). I think the issue here would rather be something like that we use a secret in the workflow file that is not specified in the fork. But either way, this is solved with the new branch.

[anonym-HPI]

@Dassderdie Do you have an idea why the workflows won't run? Maybe it's safer to first merge these changes to a branch in hpi-sam/digitalfuesimmanv where Actions will run and then merge to dev from there?

This could be, because not enough reviews approved it, maybe?

Actions always ran independent from the number of approving reviews.
Reviewers should also be able to see the results of the pipeline before approving an pull request.
Creating a new branch in this repo and merging in there also fixed this problem...

I don't know what the default is in Github, but this mechanism is available. Think about 10000 forks creating PRs and costing the repo owner money or taking away every free pipeline minute. Don't know if it could run on the forked pipeline or if it does. I saw this on another bigger repo (but then you have problems when secrets are used, as they will not be transfered to the fork), without some kind of approval (maybe some other kind of approval) or so, the pipeline didn't start.

You can specifically configure that. For this repo, an approval is required for Actions of PRs by first-time contributers (or something similar). I think the issue here would rather be something like that we use a secret in the workflow file that is not specified in the fork. But either way, this is solved with the new branch.

We can work on the new one, in the future something like that should be maybe mitigated (as this breaks a long PR with also a long review process into two)

[Dassderdie]

We can work on the new one, in the future something like that should be maybe mitigated (as this breaks a long PR with also a long review process into two)

The significantly bigger problem of this PR was that it was way too big. The commit history is also not very good, as the review processes often refactored many of the changes here. So maybe closing this PR and making a fresh start is a good thing, as this PR got very convoluted.
Another thing that should have probably been done is writing a changelog/description of what this PR actually changes. This PR doesn't only change the treatment system but also does smaller refactorings, e.g. changes the name of the male-patient.svg asset.

TLDR: There are more significant problems than creating a separate branch/PR for a fork...