ibpsa / modelica-ibpsa-mpc

Model library for MPC
Other
8 stars 1 forks source link

merge script #8

Open Mathadon opened 4 years ago

Mathadon commented 4 years ago

I have completed (the first version of) the merge script. The merge script relies on the git patch functionality, which can generate merge conflicts to sort out conflicting edits. Furthermore, I prefer not to make automated git commits. Therefore, the merge process is now split up in two steps, which consists of multiple commits and two python calls to https://github.com/ibpsa/modelica-ibpsa-mpc/blob/issue1_sources_modifications/IbpsaMpc/Resources/Scripts/merge.py.

The following instructions are generated upon calling the merge script for the first time:

[parallels@localhost IbpsaMpc]$ python Resources/Scripts/merge.py 

The merge has been succesfully prepared. The following steps still have to be completed:
1) Commit the updated files that have been copied using:
> git commit -a -m "Merged files from the library IBPSA"
2) Run the merge script again, which updates .copiedFiles.txt, upon which the merge process continues. To abort, checkout all files.

and these the second time:

[parallels@localhost IbpsaMpc]$ python Resources/Scripts/merge.py 
The merge continues. The stored sha is 1b3b2a6e23d7de10cb62634a2dea57069cc74ab9. Next steps are:
3) Reapply earlier modifications using the patch file:
> git apply --3way merge.patch
4) Resolve conflicts if required, then also commit these changes, without amending:
> git commit -a

What happens in the back-end: the file .copiedFiles contains the sha of code before the previous patch was applied. When starting the script, a patch file is generated compared to this sha and stored to file (only for the files that are listed in the script), which effectively summarizes all changes compared to last merge. After that the file selection is copied from the IBPSA library using the normal merge scripts, which overwrites custom changes that may exist in these files. The file .copiedFiles is updated, but does not contain the new sha yet. This version of the code has to be committed for the patch process to work and the sha of that commit is also saved in.copiedFiles in phase 2 of the merge script. In phase 2, the sha is amended to .copiedFiles and further instructions are printed to apply the patch, which may or may not result in merge conflicts, which must then be resolved, then committed by the developer.

This works for the checkBoundary() removal (without merge conflict) but I'm sure that corner cases will pop up. @dhblum any feedback on this?

Mathadon commented 4 years ago

Merge script code is on https://github.com/Mathadon/BuildingsPy/tree/issue278_mergeMpc

dhblum commented 4 years ago

@Mathadon I do not have any experience with the merging of the IBPSA Library into other libraries, so the code related to that, specifically in BuildingsPy, I am still learning. But from what I can tell, your suggested approach (implemented in https://github.com/ibpsa/modelica-ibpsa-mpc/pull/3) seems OK to me. I agree with not having automated git commits. This is definitely an expert-only process to embark on. But I was able to replicate the functionality using the code in your PR.

One note/emphasis is in 1) your suggested use of the -a option when committing requires the git staging area to be clear of any unwanted changes to files not associated with the IBPSA merge update.

Also, when the process is complete, should merge.patch be removed as a clean-up? Otherwise, it is left as an untracked file in the repo.

Mathadon commented 4 years ago

First comment; that is correct. I have added

We assume that the git staging area was clean before starting the process, otherwise watch out what you commit.

Second comment;

5) remove the patch file if desired. > rm merge.patch

Should I make a PR for buildingspy already or do we test it further first?

dhblum commented 4 years ago

Sounds good.

I think test a bit further before making PR for buildingspy:

1) In any case, the docstring of mergeIbpsaMpc will have to be updated, I imagine similarly to merge and your above comment with instructions could serve as a basis. 2) I don't think https://github.com/ibpsa/modelica-ibpsa-mpc/pull/3 at the moment includes parameter changes or extensions of IPBSA models. This functionality should be tested [EDIT: ... tested before making PR for buildingspy, in case it should/could be done in a separate PR than https://github.com/ibpsa/modelica-ibpsa-mpc/pull/3.

Mathadon commented 4 years ago

Let's develop it a bit further then before doing the time investment in documenting.