Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ENH: added StackExchange-style MathJax filtering #2337

Merged
merged 0 commits into from Aug 24, 2012

Conversation

Projects
None yet
6 participants
Contributor

ahmadia commented Aug 24, 2012

This pull request attempts to fix issue #2289

Owner

Carreau commented Aug 24, 2012

Hi,
Thanks. This does not merge cleanly with curent master. Do you think you could rebase it on a slightly more recent version of the source ?

Contributor

ahmadia commented Aug 24, 2012

Gross, somehow I managed to patch against a stale ipython fork on github.
I'm not going to be able to deal with this tonight, I might be able to get
to it Sunday.

A

This pull request passes (merged 43073c3 into 144f08a).

Owner

Carreau commented Aug 24, 2012

Gross, somehow I managed to patch against a stale ipython fork on github.
I'm not going to be able to deal with this tonight, I might be able to get
to it Sunday.

No problem. We like to keep PR without merge commit, so if you could do a nice rebase, it would be better :-)
Thanks.

Contributor

ahmadia commented Aug 24, 2012

A rebase would require a new pull request, right? It turns out the merge wasn't too bad.

Owner

Carreau commented Aug 24, 2012

No, a rebase can be force pushed.

$ git push -f

Even if a merge if not bad, we have release script that count the number of merged PR by counting the number of merge commits, and in case we need to git-bisect it's better to have as few merge commits as possible.

Contributor

bfroehle commented Aug 24, 2012

No, you can just rebase and push --force your changes back to github.]

Contributor

asmeurer commented Aug 24, 2012

Even if a merge if not bad, we have release script that count the number of merged PR by counting the number of merge commits, and in case we need to git-bisect it's better to have as few merge commits as possible.

Couldn't you just check for commits of the form "Merge pull request #NNN from user/branch"? And git bisect is smart enough to handle merges (even octopus merges). Discouraging merges is a bad policy IMHO (if anything, you should discourage rebasing).

@ahmadia ahmadia merged commit e32b091 into ipython:master Aug 24, 2012

1 check passed

default The Travis build passed
Details
Owner

Carreau commented Aug 24, 2012

I won't fight for what is better rebasing or merging, some argue that rebasing is easier for cherry-picking and backporting, git is great as merging so why not using it... I have no strong opinion myself.
Current politics is to avoid any upstream merge in PR and we try to stick with it as much as possible.

Contributor

ahmadia commented Aug 24, 2012

Forcing a rebased push appears to have broken github/closed the pull
request, how annoying.

The rebased tip is here:
https://github.com/ahmadia/ipython/commits/mathjax_fix

A

On Sat, Aug 25, 2012 at 12:20 AM, Bussonnier Matthias <
notifications@github.com> wrote:

I won't fight for what is better rebasing or merging, some argue that
rebasing is easier for cherry-picking and backporting, git is great as
merging so why not using it... I have no strong opinion myself.
Current politics is to avoid any upstream merge in PR and we try to stick
with it as much as possible.


Reply to this email directly or view it on GitHubhttps://github.com/ipython/ipython/pull/2337#issuecomment-8017536.

Owner

Carreau commented Aug 24, 2012

weird.
Closed PR might take more time to update when you force push, it happend to me once or twice.
Let's wait a few hours to see if it can be reopened.

Contributor

ahmadia commented Aug 24, 2012

I'm off to bed/weekend. Just to recap, this code is almost certainly not
ready to be merged in. It works against a very simple test and some larger
notebooks, but I haven't checked against a more serious example, all of the
LaTeX+Markdown examples will need to be updated, and the code should be
refactored into the right files (the functional decomposition seems fine to
me, they're just probably not organized well).

A

On Sat, Aug 25, 2012 at 12:38 AM, Bussonnier Matthias <
notifications@github.com> wrote:

weird.
Closed PR might take more time to update when you force push, it happend
to me once or twice.
Let's wait a few hours to see if it can be reopened.


Reply to this email directly or view it on GitHubhttps://github.com/ipython/ipython/pull/2337#issuecomment-8017793.

Owner

minrk commented Aug 25, 2012

Did you do a merge before trying the rebase? Because the git merge message suggests that this is what happened. This is clearly a bug in GitHub's autoclose hook, because merging master into the PR is triggering the same event as the branch merged into master. I only ask because dozens of pull requests have been rebased on master, which hasn't triggered the merge event before.

Contributor

ahmadia commented Aug 25, 2012

Sent from my iPhone

On Aug 25, 2012, at 1:04 AM, Min RK notifications@github.com wrote:

Did you do a merge before trying the rebase? Because the git merge message suggests that this is what happened. This is clearly a bug in GitHub's autoclose hook, because merging master into the PR is triggering the same event as the branch merged into master. I only ask because dozens of pull requests have been rebased on master, which hasn't triggered the merge event before.


Reply to this email directly or view it on GitHub.

I almost certainly did whatever bad thing GitHub says I did, but with the best of intentions :o)

A

Contributor

ahmadia commented Aug 27, 2012

Hey All,

Any suggestions on what to do here? I've rebased against the latest master
as a single commit and force-pushed again, but the pull request is still
"closed".

A

On Sat, Aug 25, 2012 at 10:20 AM, Aron Ahmadia aron.ahmadia@gmail.comwrote:

Sent from my iPhone

On Aug 25, 2012, at 1:04 AM, Min RK notifications@github.com wrote:

Did you do a merge before trying the rebase? Because the git merge message
suggests that this is what happened. This is clearly a bug in GitHub's
autoclose hook, because merging master into the PR is triggering the same
event as the branch merged into master. I only ask because dozens of pull
requests have been rebased on master, which hasn't triggered the merge
event before.


Reply to this email directly or view it on GitHubhttps://github.com/ipython/ipython/pull/2337#issuecomment-8018156.

I almost certainly did whatever bad thing GitHub says I did, but with the
best of intentions :o)

A

Owner

Carreau commented Aug 27, 2012

Open a new one then, an reference this one.
You can also send a ticket to github. They are usually fast to respond.

Contributor

asmeurer commented Aug 27, 2012

In my experience it's impossible to reopen a pull request that was closed automatically. This is done intentionally, because sometimes people reuse branch names, and they wouldn't want old PRs being reopened.

Owner

Carreau commented Aug 27, 2012

This is done intentionally, because sometimes people reuse branch names, and they wouldn't want old PRs being reopened.

This hold only for "auto-reopened" PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment