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

Improve instructions how to use private/local directory #13381

Closed
wants to merge 1 commit into from
Closed

Improve instructions how to use private/local directory #13381

wants to merge 1 commit into from

Conversation

dalanicolai
Copy link
Contributor

Improve instructions how to use private/local directory. This is a new pull request that replaces #13209.
In this pull request I implemented the suggestions and feedback given in that pull request.
After this one is accepted please close pull request #13209.

private/local/README.md Outdated Show resolved Hide resolved
private/local/README.md Outdated Show resolved Hide resolved
private/local/README.md Outdated Show resolved Hide resolved
private/local/README.md Outdated Show resolved Hide resolved
private/local/README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dalanicolai,

I think duianto requested some changes for your PR. Please review and fix them.
Btw: Please push your changes directly to your fork's branch overwriting your previous commit, this will automatically update your PR.

@dalanicolai
Copy link
Contributor Author

@smile So I don't know how this, works. The PR still shows waiting for response, and Changes requested. But I have processed all suggestions. Guess this PR should be ready now

Copy link
Collaborator

@duianto duianto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label Waiting for response and the requested review are changed manually by the Spacemacs maintainers.

We prefer one commit per PR, unless it's a complex PR and in that case it might be best to open multiple PRs. But in this case the changes are simple so let's squash them.

Since you mentioned that your new to git: #13377 (comment)

These instructions might be helpful.

To squash commits with magit.
In the magit: <repository name> buffer:

If your new to squashing commits, then it's best to create a new temporary branch where you can experiment. I called it: testing-13381-squash-commits

  • Now press ri to rebase interactively
    A magit-log-select: <repository name> buffer opens, with a list of the most recent commits.
  • Move the cursor to the oldest commit, that you want to squash the newer commits into.
    When these instructions were written, then this PRs commits are:
d216a8410 * testing-13381-squash-commits fix PR
2bbff4f86 * Apply suggestions from code review
2a1a36120 * Improve instructions how to use private/local directory

Here we move the cursor to the oldest commit:
2a1a36120 * Improve instructions how to use private/local directory
The top of the buffer shows the key bindings:
C-c C-c to rebase and C-c C-k to abort.
In Spacemacs with the evil key bindings, it also works to press:
,, (that's comma comma) or ,c to rebase
,a or ,k to abort

  • Press ,, (or C-c C-c if your using the Emacs key bindings) to start rebasing
    A git-rebase-todo buffer opens and it lists the commit that the cursor was on and any newer commits:
pick 2a1a36120 Improve instructions how to use private/local directory
pick 2bbff4f86 Apply suggestions from code review
pick d216a8410 fix PR

Notice that the commits are listed in the opposite order (oldest to newest).
An explanation here describes it like this:
https://www.reddit.com/r/git/comments/agbpfa/git_interactive_rebase_order_is_wrong/ee50a0p/

git log displays the latest commit first because it's likely to be the most relevant, and from a UI perspective it fits in better with the command line model.

git rebase -i displays the oldest at the top because your editor cursor starts at the first line from the top, and it makes sense to work your way through a series of changes chronologically when recalling what each change did.

You can see comments at the bottom of the git-rebase-todo buffer about what you can do with the commits.
We're interested in the line:
# s squash = use commit, but meld into previous commit

  • Move the cursor to the second line:
    pick 2bbff4f86 Apply suggestions from code review

  • Press s to change it to:
    squash 2bbff4f86 Apply suggestions from code review

  • Move the cursor to the third commit
    pick d216a8410 fix PR

  • Press s to change it to:
    squash d216a8410 fix PR

  • Squash the commits by pressing ,, (or C-c C-c with the Emacs key bindings)
    A new buffer opens: ~/.emacs.d/.git/COMMIT_EDITMSG
    it lists the subjects (titles) and body messages of all of the rebased commits.
    Edit them as appropriate.

  • Press ,, (C-c C-c with the Emacs key bindings) to commit the changes.

Now the test branch only contains a single commit with the changes of all of the previous three commits.

You can practice it again by checking out your local branch of this PR and creating a new test branch. Or if your "comfortable" with squashing, then you can squash the commits on your local branch of this PR.

Then you can replace this PRs multiple commits by pushing to your Spacemacs fork on GitHub with the argument: force with lease:

  • Press p to open the Magit Push transient popup,
  • Press -f (- dash and f) to enable the --force-with-lease argument.

If your Spacemacs fork on GitHub is set as the remote.pushDefault.

You can check what it's set as in the:
b branch transient popup, Under the Variables section

For me it shows:
p branch.develop.pushRemote [checkversion|fork|origin|remote.pushDefault:fork]
The last entry: remote.pushDefault:fork has a green color, meaning that it's selected.

The p push transient popup section: Push <branch name> to
shows:
p fork/<branch name>
u origin/ e` elsewhere

If you don't have the p target pointing to your Spacemacs GitHub fork, then you can press e to choose the target branch.

When you push to your Spacemacs GitHub fork, then this PRs commits will be force updated and the current multiple commits will be replaced by the (squashed) single commit.

private/local/README.md Outdated Show resolved Hide resolved
@nixmaniack
Copy link
Contributor

@duianto Those are quite detailed instructions on rebasing using magit 👍 and they look like can be added to wiki for future references.

@smile13241324 smile13241324 self-requested a review March 24, 2020 20:50
Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should still double quote the path in the load example. The rest is ok now.

@dalanicolai
Copy link
Contributor Author

@duianto Thanks for the great instructions! Definitely nice exercise and I guess it prevented some confusion also... I hope;) @nixmaniack @smile13241324 thanks for the reviews... Anyway, did as you said and I guess now the pull request is ready...

@duianto
Copy link
Collaborator

duianto commented Mar 26, 2020

@nixmaniack Thanks for the suggestion.
I opened a PR:
CONTRIBUTING.org add a link to squash instructions #13420
that adds a link to the comment above. To the end of the line:

If you still have several commits, squash them into only one commit (here's a guide)

https://github.com/syl20bnr/spacemacs/blob/develop/CONTRIBUTING.org#ideally-for-simple-prs-most-of-them

@smile13241324
Copy link
Collaborator

Looks good will be merged soon.

@smile13241324
Copy link
Collaborator

Thank you for contributing to spacemacs 💜, I have cherry picked your commit into develop.

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