redguardtoo / js-comint

js-comint will send the code from Emacs into node.js or rhino
GNU General Public License v3.0
69 stars 22 forks source link

Remove references to inferior-js-process-output #20

Closed wyuenho closed 6 years ago

wyuenho commented 6 years ago

This is already setup when the mode activates, so it's no longer necessary to setup manually.

redguardtoo commented 6 years ago

Could you merge all the pull request into one request containing one commit?

Besides, maybe you could use abo-abo's way to upgrade package? https://oremacs.com/2015/01/27/my-refactoring-workflow/

wyuenho commented 6 years ago

They are all separate issues, I’d prefer to keep them separate so we can track changes easier. Also I doubt anyone is using js-cominit-filter-output or you would have noticed it by now. Otherwise, please feel free to edit the PRs as you see fit.

On 5 Nov 2017, at 12:07 am, Chen Bin notifications@github.com wrote:

Could you merge all the pull request into one request and containing one commit?

Besides, maybe you could use abo-abo's way to upgrade package? https://oremacs.com/2015/01/27/my-refactoring-workflow/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

redguardtoo commented 6 years ago

No problem. One pull request with separate commits is fine (at least merge your pull requests). But make sure you use abo-abo's way when changing function name.

wyuenho commented 6 years ago

done

redguardtoo commented 6 years ago

Just double check. patch-4 branch includes all your commits?

wyuenho commented 6 years ago

Just click the merge button on all 4 PRs

On 5 Nov 2017, at 3:31 am, Chen Bin notifications@github.com wrote:

Just double check. patch-4 branch includes all your commits?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

wyuenho commented 6 years ago

@redguardtoo Are you merging the PRs?

redguardtoo commented 6 years ago

把pull request合并成一个pull request,一个pull request可以有多个commit。 可以使用 git rebase命令合并。

我是对自己负责的开源项目都要审核代码的,不可能只按那个绿色的按钮。你每个commit都来个pull request我工作量吃不消(每个pull request都可能有潜在的代码冲突,你一个commit一个pull request,岂不是要我每个commit都跑一次mergetool?)。

wyuenho commented 6 years ago

The convention on Github is one PR per issue, so it makes searching for past changes easier for people. Conflicts only happen if 2 PRs edited the same lines. This is the only PR that will result in conflicts, in which I will rebase after you've merged the previous PRs. Fixing conflicts is my responsibility, not yours, although you can if you want. You don't need to check out the PRs and look at your merge tool, this hasn't been necessary for many years now, just look at the diffs on Github for each PR, if it looks ok, click the merge button.

If you don't have the time, I'd be happy to takeover maintaining this package.

redguardtoo commented 6 years ago

You misunderstood the GitHub convention. A pull request corresponds to an independent a task, usually take a senior developer 0.5~1 day to finish. In your case, the four commits could be merged into just a code cleaning task. So it's better to merge them into one pull request. Create a new branch and run cherrypick or rebase to include all these commits.

wyuenho commented 6 years ago

Given the poor state of this project and disagreement with the way you work, I've lost all confidence in this project. I've switched to nodejs-repl instead.