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

Preserve attrs with imports_granularity=Item #5233

Closed
wants to merge 1 commit into from

Conversation

9999years
Copy link

@9999years 9999years commented Feb 21, 2022

Closes #5030

On the road to #4991

@9999years 9999years marked this pull request as ready for review February 21, 2022 19:14
@ytmimi
Copy link
Contributor

ytmimi commented Feb 21, 2022

Thanks for the PR!

looks like a pretty simple fix to just pass the attributes along, which is great. When you have a chance, could you add test cases for the other variants: Preserve, Crate, Module, Item, One.

@9999years
Copy link
Author

#5030 doesn't concern the other variants, which have harder-to-fix bugs. I'll see if I can work on them but it'll be in a separate PR.

@calebcartwright
Copy link
Member

Thank you for the PR! This seems to be a duplicate of #5031 to me though, and the interactions with the other variants was one of the outstanding questions still unresolved over there (#5031 (comment))

As such, I think it' be best to either extend the resolution as suggested in #5031 or as @ytmimi suggested, add some cases for other variants to make sure there's no unanticipated side effects.

I don't recall offhand whether the diff is currently on a call path that's only reachable with the Item variant, but this portion of the codebase gets refactored frequently enough that the tests are still valuable even if only as a preventative/defensive measure

@calebcartwright
Copy link
Member

Closing in favor of #5314

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

Successfully merging this pull request may close these issues.

imports_granularity=Item removes #[cfg()] attribute from imports
3 participants