OutpostUniverse / OP2Archive

Console Application for examining, creating, and extracting files from Outpost 2 .vol and .clm archives.
https://wiki.outpost2.net/doku.php?id=outpost_2:helper_programs:op2archive
MIT License
1 stars 0 forks source link

Standardize For Loops #9

Closed Brett208 closed 6 years ago

Brett208 commented 6 years ago

Closes #2 Closes #8

Brett208 commented 6 years ago

I'm not exactly sure what happens here. OP2Archive is updating to branch Standardize For Loops in OP2Utility. So when the branch is merged and deleted in OP2Utility, I'm not sure where that leaves this pull request. I'm guessing we need to go in and re-update this one to OP2Utility Master at some point?

-Brett

Brett208 commented 6 years ago

I'm not really sure how to reference a specific commit hash. Would that mean when fetching code, OP2Archive would be smart enough to grab the master branch if the commit was moved from the temp branch to the master?

Other thing I wonder is if rebase and merge changes the commit hash and maybe needs to be avoided in these situations?

For now I'll merge OP2Utility into the master first, then update this branch to the master of OP2Utility and then merge it. Lets try targeting the hash on the next branch that it comes up on though.

Thanks for the review, -Brett

DanRStevens commented 6 years ago

How are you specifying the submodule? I assume initial setup needs a repo, and from then on a specific commit must be referenced on each update. That commit might be through a symbolic name, such as the branch name, or HEAD, or through the SHA. I'm saying it should be specified with an SHA rather than a branch name. Ideally, the SHA should be on master.

If you did a rebase and merge, it may rewrite history, which may change the commit SHA. I believe the SHA may be preserved if nothing else has been pushed to master since either the branch point or the most recent rebase. If you do a squash and merge, the SHA should be preserved if nothing else has been pushed to master since either the branch point or the most recent rebase, and the branch contains only a single commit. If you do a merge commit, that SHA should be preserved on master, but will be down one of the forked paths, unless a fast-forward merge was done, which I don't think is default behaviour on GitHub. For a merge commit, I think it would be more proper to wait until the merge was actually done, and then use the SHA of the merge commit itself, rather than the commit before which is being merged into master. If you use the branch SHA, you may miss changes that were incorporated into master after the branch was started or last rebased.

In short, you should be merging into master before updating a submodule reference to point at the new code.

Brett208 commented 6 years ago

Makes sense. Thanks.