-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[No QA] Remove all inline named exports. Update style guide. #6013
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Project built fine and looks fine.
NAB: Since all exports should be at the bottom of the file, including default
, we should update other places of the documentation like (maybe is the only one :) ):
Update: Changed my comment into a NAB because I see that the code in all xx.stories.js
don't have the default export at the end. Maybe you are intentionally leaving it out of the refactor?
Honestly, I skimmed about half of the files and didn't see any so I think we can fix them as we go. |
It's fine, it can be fixed as we go, as you say. In case you are curious, this is one of the files I meant: App/src/stories/HeaderWithCloseButton.stories.js Lines 9 to 12 in 5f47e12
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, it builds fine and I didn't notice anything no working in the app.
Leaving it un-merged in case @roryabraham wants to review.
Went through them all. I think we got em! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🚀 Deployed to staging by @roryabraham in version: 1.1.8-14 🚀
|
🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀
|
cc @roryabraham
Details
Removes all inline named exports
Fixed Issues
$ #6011
Tests / QA Steps
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android