Skip to content

Conversation

@davecampbell
Copy link
Contributor

someone mentioned an opt-in directory, thought that was a good idea.
also a convenient place to let people have their own 'personal page' if they don't have a LinkedIn or even if they do.
i tried to keep everything to the vuepress pattern.

@jsquyres
Copy link
Contributor

You might want to squash your commits down so that your email doesn't appear in git history.

@davecampbell
Copy link
Contributor Author

You might want to squash your commits down so that your email doesn't appear in git history.
i hadn't thought of that.
is squash a git option or do you mean just consolidate the actual commit to be the desired end result?
i did those last changes directly in github so it made a separate commit for each change.
i'll be glad to basically re-do the whole PR if that is best approach.
thanks for the feedback.

@InfoSec812
Copy link
Contributor

Easy peasy:
image

@davecampbell
Copy link
Contributor Author

the learning never stops!

@jsquyres
Copy link
Contributor

I'm not a big fan of Github's "Squash and merge" functionality, because:

  1. You lose the history of where the PR branch was originally created from.
    • In this case, it likely doesn't matter, but in larger projects, knowing where PR branches came from can help track down subtle git merge/resolution errors.
  2. Many people are lazy and don't edit the commit message in the Github UI to make a new and reasonable combined git commit message for the new squashed commit.
    • I realize that this is effectively a toy git repo 😄 -- it's pretty small and simple. But we should at least try to encourage good Git behavior, especially for those who are learning.

Also, @davecampbell doesn't see that Github option because he doesn't have merge perms on this repo.

I usually use git rebase -i ... to slice and dice my commits (e.g., to squash multiple commits down to a smaller number of commits): it's called interactive rebasing. Google for "git interactive rebasing" for lots of tutorials, blog posts, etc.

@davecampbell
Copy link
Contributor Author

great feedback - i'll use this as an opportunity to learn more about rebase-ing - i know it's a useful feature and i've never felt comfortable doing anything but the standard workflow.

@jsquyres
Copy link
Contributor

jsquyres commented Jun 16, 2022

i'll be glad to basically re-do the whole PR if that is best approach.

@davecampbell Nah, it's rarely necessary to close a PR and open a new one. Instead, I just edit the commits on the PR branch and then force push back up to Github. Github will then automatically update the PR to show the current set of commits on the PR branch (even if you have totally changed those commits).

Here's an example from the Open MPI repo from yesterday: open-mpi/ompi#10473 (comment). A user submitted a PR with a few needless commits, and at least one of them wasn't signed off properly (Open MPI requires the Linux kernel-style "Signed-off-by ..." line in commit messages). I advised the user to squash down and sign properly; he did so and force-pushed back to his PR branch. End result (i.e., as of open-mpi/ompi#10473 (comment)): the PR now has 1 commit that is signed off properly, and the CI test checking for the signoff message now passed.

@davecampbell
Copy link
Contributor Author

ok - i think i accomplished that task.
that is very useful, but wow, i can see where things get uber-complicated if a lot of those shenanigans come into play.

@jsquyres
Copy link
Contributor

ok - i think i accomplished that task.

Nicely done! 👍

@jsquyres jsquyres merged commit a117096 into KYOSS:main Jun 17, 2022
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.

3 participants