Merge documentation

[carmenromo]

Documentation has been included with the instructions to merge according to our requirements in IC.

[jacg]

Probably should mention --no-ff and -n.

[carmenromo]

Do you think I should mention them here or in the commands instructions?

[jacg]

This is the overview. Details such as these should go in the detailed mechanics section.

[jacg]

Oh, and @halmamol and @MiryamMV are, right now, the most qualified people on Earth to comment on what can/needs to be improved here!

[MiryamMV]

I'd include the explicit instructions for Magit (or CL). Perhaps they're not necessary for the experienced git users; I'm no expert and I'd appreciate the Magic/CL hints, specially for the merge and "author changing" bits.

[jacg]

I'd include the explicit instructions for Magit (or CL).

On the one hand, I agree; on the other, this is a description of the expectations of merges in the IC repo, rather than a Git tutorial.

By adding many details about how Git is used, we dilute the message about IC merge requirements.

Perhaps we should do the following:

• What is there so far, is not bad at all and provides a decent overview of the steps that need to be taken. Leave it as it is (apart from minor cosmetic changes).

• Add a subsequent section giving detailed instructions for (some of) the steps, with references to these, from the corresponding parts of the overview.

[carmenromo]

On the one hand, I agree; on the other, this is a description of the expectations of merges in the IC repo, rather than a Git tutorial.

Yes, I agree. I was not sure about all the explicit instructions, but it's true that it was for me an extremely useful tool.

Add a subsequent section giving detailed instructions for (some of) the steps, with references to these, from the corresponding parts of the overview.

Nice! I'll do it

[halmamol]

I think adding the magit commands would be useful, I'm still not too familiar with magit. The rest is fine! Thanks a lot @carmenromo !!

[jacg]

For some reason I can't reply to the following in place:

Does this mean 'set the reviewer as the author of merge commit'?

Yes.

[jacg]

Looks good.

I'd make it clear at the top that there are some detailed instructions at the bottom.

I'm not entirely sure about the correctness of all the commands, off the top of my head (playing them through in my head seemed to make sense), and I don't have the time to simulate the situation and check them all. If nobody else spots any mistakes, I suggest we go ahead. I suggest that the mergers try to verify the instructions during the next few merges they do. (Don't forget that you can see the rendered version of the document in the GitHub interface when looking at this branch, even before it is merged.)

The trouble is that, here, we're programming humans, so we can't write automated tests :-)

[halmamol]

I'm not entirely sure about the correctness of all the commands, off the top of my head (playing them through in my head seemed to make sense), and I don't have the time to simulate the situation and check them all. If nobody else spots any mistakes, I suggest we go ahead. I suggest that the mergers try to verify the instructions during the next few merges they do. (Don't forget that you can see the rendered version of the document in the GitHub interface when looking at this branch, even before it is merged.)

Would you prefer that @MiryamMV or I (who ever takes care of the next merge) uses this version as reference during the next merge before its approval? It would be the best way to cross check all the commands.

[jacg]

Would you prefer that @MiryamMV or I (who ever takes care of the next merge) uses this version as reference during the next merge before its approval?

I was going to suggest this, but I was afraid that no new PR might come along for a while, and then we'd be left waiting. Adding a fix later, seemed the lesser evil. But if nobody objects to waiting until the new mergers have verified it on real PRs a few times, then I'm all for it.

[halmamol]

I was going to suggest this, but I was afraid that no new PR might come along for a while, and then we'd be left waiting. Adding a fix later, seemed the lesser evil. But if nobody objects to waiting until the new mergers have verified it on real PRs a few times, then I'm all for it.

If you think that next PR is going to be in a while, then these instructions would be even more necessary. I wouldn't mind waiting. Up to you!

[carmenromo]

I'm not entirely sure about the correctness of all the commands

I doubt in the CLI commands, the Magit ones I checked in there while I was writing them. But nice anyway if @halmamol and @MiryamMV make a second check!

If you think that next PR is going to be in a while, then these instructions would be even more necessary. I wouldn't mind waiting. Up to you!

I wouldn't mind either!

[carmenromo]

I'd make it clear at the top that there are some detailed instructions at the bottom.

Done!

[gonzaponte]

#785 is a tiny PR that could probably be reviewed, approved, and merged very quickly. It could be used for checking this documentation, but it needs a reviewer. I think my request for new reviewers can be declared a failure, so maybe @jacg or @mmkekic can take care of it?

[jacg]

If you think that next PR is going to be in a while, then these instructions would be even more necessary.

... which is why I pointed out that these instructions are already readable in their beautifully-rendered glory in ... oh, let me be very specific this time ... right here.

That is to say, we don't need to merge this PR for its content to be available to the mergers. So we can take our sweet time in merging it, even if we do want to merge it eventually.

[halmamol]

Since we prefer Magit, we have revised the CLI with exception of the merge (we had a small issue and preferred to do it with Magit). Everything works fine!

[carmenromo]

Since we prefer Magit, we have revised the CLI with exception of the merge (we had a small issue and preferred to do it with Magit). Everything works fine!

Nice! So, if @jacg agrees, this can be approved and merged!

[jacg]

Fine by me. (My submit your review button seems to have gone AWOL, so I can't give a formal approval.)

[jacg]

Four people have pored over this a number of times, and the two new mergers have even performed some merges while verifying the instructions, so I think we can consider this to be as thoroughly tested as instructions-for-humans could reasonably be.

[jacg]

[Second attempt at formal approval]

Four people have pored over this a number of times, and the two new mergers have even performed some merges while verifying the instructions, so I think we can consider this to be as thoroughly tested as instructions-for-humans could reasonably be.