-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
refactor: introduce Document #876
Conversation
7e88ce7
to
ad1c680
Compare
ad1c680
to
603fab0
Compare
I am a bit concerned about releasing breaking changes in 5.1, is this necessary? Could we hold it off for a while until we have some bigger features which might warrant a 6.0? Or could it be made not breaking, by returning the thing we used to return too and deprecating it for now? |
Yeah, I can easily make it a non-breaking one and keep it like that. |
Great ok!
…On Mon, Dec 30, 2019 at 18:43, Marc MacLeod ***@***.***> wrote:
@marbemac commented on this pull request.
---------------------------------------------------------------
In [src/functions/unreferencedReusableObject.ts](#876 (comment)):
> @@ -1,5 +1,6 @@
+import { isObject } from 'lodash';
Yeah it's not worth it - in order to make the lodash/isObject type syntax work, we'd have to use that syntax consistently everywhere, and hope that our dependencies do as well. Better to try to get tree shaking working, but even then it's not a huge difference in output bundle size.
—
You are receiving this because your review was requested.
Reply to this email directly, [view it on GitHub](#876?email_source=notifications&email_token=AAAQONJVRCV2KJOP2VWXJN3Q3IXLFA5CNFSM4J6ZAPTKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQMQQJQ#discussion_r362051001), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAQONJCS7XK3MRD2S3M4KTQ3IXLFANCNFSM4J6ZAPTA).
|
Checklist
Does this PR introduce a breaking change?
Additional context
Technically this is a breaking change, but I doubt anyone besides us actually passesnot breaking anymore 😊IParsedResult
, so we can consider it a fully Internal refactor that mostly addresses the issues with naming and makes certain parts of code a bit easier to reason about.I'm not planning to make it a part of 5.0.