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

Rebase of PR #794 (fixes #793) #907

Closed
wants to merge 6 commits into from
Closed

Conversation

SpainTrain
Copy link

@SpainTrain SpainTrain commented Aug 2, 2017

#794 appeared stalled, so I just updated it. LMK what to do RE changelog and squashing.

@ephys, LMK if you rebase #794 and I can just close this (thank you for creating this, I just didn't want to see it languish :) ).

Closes #793


This change is Reviewable

ephys and others added 6 commits April 6, 2017 15:57
* origin/master: (66 commits)
  [Fix] unescape unnecessarily escaped regex slashes
  [Dev Deps] dev dep ranges should match peer dep ranges
  docs(readme): add space (import-js#888)
  bump to v2.7.0
  changelog note for import-js#843
  upgraded no-absolute-path to use `moduleVisitor` pattern to support all module systems
  PR note fixes
  bump to v2.6.1 to bump dep on node resolver to latest 😳
  Update no-extraneous-dependencies.md (import-js#878)
  Fix flow interface imports giving false-negative with `named` rule
  bump to 2.6.0, node/0.3.1, webpack/0.8.3, memo-parser/0.2.0
  chore(eslint): upgrade to eslint@4
  memo-parser: require eslint >= 3.5.0 (need file path always)
  build on node v4, again (import-js#855)
  bump to v2.5.0
  bump `debug` version everywhere
  resolvers/webpack: v0.8.2
  eslint-module-utils: v2.1.1 (bumping to re-publish to npm)
  [Tests] comment out failing (and probably invalid) test
  Only apps should have lockfiles.
  ...
@coveralls
Copy link

coveralls commented Aug 2, 2017

Coverage Status

Coverage increased (+0.002%) to 95.967% when pulling db253b7 on SpainTrain:pr-794 into dd28130 on benmosher:master.

@ljharb
Copy link
Member

ljharb commented Aug 2, 2017

It's not a good idea to open a replacement PR; if the collaborators want to continue it, we can push directly to that PR. Opening up a new one just clutters up the repo. A comment on the original PR would have been better.

@SpainTrain
Copy link
Author

@ljharb

In general, yes I agree. However there was a comment 23 days ago and a comment similar to your suggestion 15 days ago in that PR,which neither have been responded to. I understand maintainers are busy, so this was an attempt to minimize work for others. For organizational purposes, I thought this was pretty well titled to indicate that it is an intentional dupe.

I am happy to follow house rules an any repo, but tend towards taking (even a small bit) of work off others' plate rather than add noise to a discussion.

Thanks!

@ephys
Copy link
Contributor

ephys commented Aug 2, 2017

My apologies for my lack of response, what should I do to help sort this out? :)

I'm happy to close my PR in favor to this one, or merge your PR with mine. Whichever solution works best for you

@SpainTrain
Copy link
Author

@ephys the rebase is easy, so probably best all around to rebase yours if you can? Thanks again.

@SpainTrain SpainTrain closed this Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

import/order fails after updating a dependency (redux-saga)
4 participants