Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge script #8

Open
Mathadon opened this issue Nov 20, 2019 · 5 comments
Open

merge script #8

Mathadon opened this issue Nov 20, 2019 · 5 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@Mathadon
Copy link
Member

Mathadon commented Nov 20, 2019

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 Mathadon added the documentation Improvements or additions to documentation label Nov 20, 2019
@Mathadon Mathadon self-assigned this Nov 20, 2019
@Mathadon
Copy link
Member Author

@dhblum
Copy link
Collaborator

dhblum commented Dec 4, 2019

@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 #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
Copy link
Member Author

Mathadon commented Dec 5, 2019

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;

  1. 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
Copy link
Collaborator

dhblum commented Dec 5, 2019

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 removed checkBoundary for #1 #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/removed checkBoundary for #1 #3.

@Mathadon
Copy link
Member Author

Mathadon commented Dec 5, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants