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

Length unit #196

Merged
merged 3 commits into from
Nov 17, 2020
Merged

Length unit #196

merged 3 commits into from
Nov 17, 2020

Conversation

kvid
Copy link
Collaborator

@kvid kvid commented Nov 16, 2020

@formatc1702 formatc1702 mentioned this pull request Nov 16, 2020
@formatc1702 formatc1702 added this to the v0.3 milestone Nov 16, 2020
@formatc1702
Copy link
Collaborator

Thanks.
I copied it into my local pr/196 branch for squashing, to make sure authorships were preserved as expected before committing to dev.
By not using the GitHub GUI to merge, it might appear that this PR was not merged, even though the changes have made it into dev.
I hope it's not a big deal; this is an attempt at figuring out how to deal with multiple authors of one commit :-)
See the final result in dec64ab:

image

@kvid
Copy link
Collaborator Author

kvid commented Nov 16, 2020

Thanks.
I copied it into my local pr/196 branch for squashing, to make sure authorships were preserved as expected before committing to dev.
By not using the GitHub GUI to merge, it might appear that this PR was not merged, even though the changes have made it into dev.
I hope it's not a big deal; this is an attempt at figuring out how to deal with multiple authors of one commit :-)

@formatc1702, Are you not able to add Co-authored-by in the GitHub GUI when preparing the common commit message for a squash merge-in?

If not, it might be an option to use the GitHub GUI to merge-in to a different branch, e.g. a new dummy length_unit3 branch to mark the PR as merged in afterwords (i.e. now)?

@formatc1702 formatc1702 reopened this Nov 17, 2020
@formatc1702 formatc1702 changed the base branch from dev to temp/length-unit November 17, 2020 07:23
@formatc1702 formatc1702 merged commit 3c205fe into wireviz:temp/length-unit Nov 17, 2020
@formatc1702
Copy link
Collaborator

formatc1702 commented Nov 17, 2020

Good idea with the dummy branch! Thanks.

I could have done it in the GitHub GUI (just like I did now), but I wasn't sure it would work as intended, so I decided to do it locally, and didn't realize I could not just manually mark the PR as merged afterwards.

And as I just realized, when doing it via GitHub, you are marked as the main author (see 3c205fe *, will delete later); whereas I interactively rebased everything by squashing onto zombielinux' commit, making him the main author (see dec64ab), which is closer to my intention, since he kick-started this chain of PRs.

[*] I just copy&pasted the commit message from dec64ab, that's why zombielinux is missing entirely in 3c205fe

Anyway, now I know, for next time!

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

Successfully merging this pull request may close these issues.

4 participants