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

Revert d5d44a6 to fix regression #165

Merged
merged 1 commit into from Dec 17, 2020
Merged

Revert d5d44a6 to fix regression #165

merged 1 commit into from Dec 17, 2020

Conversation

conartist6
Copy link
Collaborator

Fixes #164, a regression of kentcdodds/import-all.macro#7.

We need a test for the regression so that it doesn't break again, and (assuming this is still the right fix for the original issue) also we will have to find a different fix for the issue I have reopened, #121.

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #165 (caa5e74) into master (ccc43ec) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #165   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          131       130    -1     
  Branches        38        38           
=========================================
- Hits           131       130    -1     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccc43ec...caa5e74. Read the comment docs.

@ben-rogerson
Copy link

Hey Kent, it'd be awesome if you could patch this in - it'll fix some errors I'm seeing in twin.macro.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Sorry, this feel through the cracks for me. Thanks!

@kentcdodds
Copy link
Owner

Oh, it's still a draft PR. I think @conartist6 wanted to write a regression test for it. @ben-rogerson world you like to help with that yourself?

@conartist6 conartist6 marked this pull request as ready for review December 17, 2020 13:50
@conartist6
Copy link
Collaborator Author

conartist6 commented Dec 17, 2020

It's mergeable, with or without the regression test. The fact that every set of pushed changes is release though makes it harder for me to imagine that the test will be added in the future if it isn't added now though.

@kentcdodds
Copy link
Owner

I definitely don't have time to add a test for it myself. I'm pretty much at the "quick review and merge if people say it's ok" stage of maintaining this package 😬 (https://kcd.im/no-time).

@conartist6
Copy link
Collaborator Author

@ben-rogerson Do you mind contributing that fix? I may have time to do it over the next few days, but also I may not. You're the only one who seems to be directly affected by the possibility of a regression. If you'd rather just get the fix in now, I'm fine with that.

@kentcdodds
Copy link
Owner

We don't have many changes in this repo, so I think I'll go ahead and merge this now. @ben-rogerson if you'd like to make sure this never breaks in the future, please contribute a test. Thanks!

@kentcdodds kentcdodds merged commit 18d79d3 into master Dec 17, 2020
@kentcdodds kentcdodds deleted the revert-traverse branch December 17, 2020 15:47
@github-actions
Copy link

🎉 This PR is included in version 3.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Duplicate declaration" error due to identifier traversal change
3 participants