Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Chore: Remove es6-map and es6-weakmap as they are included in node4 #10

Merged
merged 1 commit into from
Feb 11, 2017
Merged

Chore: Remove es6-map and es6-weakmap as they are included in node4 #10

merged 1 commit into from
Feb 11, 2017

Conversation

corbinu
Copy link
Contributor

@corbinu corbinu commented Feb 10, 2017

Remove es6-map and es6-weakmap as they are included in node4

@JamesHenry
Copy link
Member

This LGTM, will do a proper review as soon as we get the node versions for the travis build sorted

@corbinu
Copy link
Contributor Author

corbinu commented Feb 10, 2017

@JamesHenry Thanks ya #11 just needs to land first

@corbinu corbinu changed the title Remove es6-map and es6-weakmap as they are included in node4 Chore: Remove es6-map and es6-weakmap as they are included in node4 Feb 11, 2017
@JamesHenry
Copy link
Member

Do you need help with rebasing? You are still doing merge commits into the other branches and so we are ending up with very messy commit histories on these PRs

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

@JamesHenry Yes please sorry I was doing "git rebase --interactive" and squashing down to one commit but then every time for some reason the merge ends up at the end not sure why

@JamesHenry
Copy link
Member

JamesHenry commented Feb 11, 2017

I am definitely not a git expert, but git rebase --interactive has always worked out well for me.

What I tend to do is:

  1. Make sure you have the eslint repo as a secondary remote before starting - in your case you are working on a fork, so your origin is based on that. I usually call my second remote which points at the original remote repo upstream, so in your case, I would have origin which is corbinu/eslint-scope and upstream which is eslint/eslint-scope

  2. Fetch the absolute latest from eslint/eslint-scope by doing git fetch upstream (given your secondary remote is set up as outlined in (1))

  3. Do an interactive rebase for your current branch against that latest by doing git rebase -i upstream/master

  4. Squash/drop/rename commits as appropriate so that you end up with a single commit which summarises all of your work which should go in the PR

As I say, I am definitely not an expert, but that method works well for me when I am working on forks.

Hope that helps!

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

@JamesHenry Strange I am pretty sure thats what I am doing .. I have my orginal brach (yes I know I misspelled it) that points to eslint-scope.git#master

I switch to that orginal branch. Do "git pull" then I switch to the branch I am working on and "git rebase --interactive orginal" and squash it down.

@JamesHenry
Copy link
Member

What is the output when you run git remote -v?

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

origin git@github.com:corbinu/eslint-scope.git (fetch)
origin git@github.com:corbinu/eslint-scope.git (push)
original git@github.com:eslint/eslint-scope.git (fetch)
original git@github.com:eslint/eslint-scope.git (push)

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

Ok I tried it again but then I end up with this

To github.com:corbinu/eslint-scope.git
 ! [rejected]        node-4 -> node-4 (non-fast-forward)
error: failed to push some refs to 'git@github.com:corbinu/eslint-scope.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details

and I when I do get pull thats when that merge gets created

@soda0289
Copy link
Member

git push --force

@JamesHenry
Copy link
Member

Is that when you are pushing after your rebase?

If so that is perfectly normal, you have to force push after the rebase. You need to do git push -f

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

Ahhh ok great thanks!

@soda0289
Copy link
Member

You should probally prefix the commit message with Chore.
You can do a git commit --amend then do another git push --force

@JamesHenry
Copy link
Member

JamesHenry commented Feb 11, 2017

Ah, @corbinu you see now that you have rebased that other work containing the travis yaml file has disappeared from this PR! This is why the rebasing is so important. Let's reopen that other PR and get that merged in first

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

@soda0289 Thanks did on the babel removal will change this one when I rebase on that

@JamesHenry
Copy link
Member

@corbinu The other PR has been merged, this is now ready to be rebased (again 😄 )

@corbinu
Copy link
Contributor Author

corbinu commented Feb 11, 2017

@JamesHenry Good to go thanks!

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nzakas nzakas merged commit 7d23f8e into eslint:master Feb 11, 2017
@corbinu corbinu deleted the remove-es6-map branch February 16, 2017 18:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants