-
Notifications
You must be signed in to change notification settings - Fork 643
Remove unnecessary this.get()
calls
#2776
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
Conversation
@@ -26,7 +26,7 @@ export default Controller.extend({ | |||
keywords: alias('crate.keywords'), | |||
categories: alias('crate.categories'), | |||
isOwner: computed('crate.owner_user', 'session.currentUser.id', function () { | |||
return this.get('crate.owner_user').findBy('id', this.get('session.currentUser.id')); | |||
return this.crate.owner_user.findBy('id', this.session.currentUser?.id); |
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.
none of these are async relationships?
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.
hmm, good question, some of them actually are 🤔
☔ The latest upstream changes (presumably #2802) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
@Turbo87 you have some conflicts here |
yeah, I'm aware. closing this for now :) |
4eb20de
to
60cfbcf
Compare
@locks I've rebased this and checked the references to async relationships. Those that are relevant are covered by tests and are still working fine after this change. |
Looking sharp! @bors r+ |
📌 Commit 21d2bc3 has been approved by |
☀️ Test successful - checks-travis |
These should not be necessary anymore AFAICT
Resolves #1795
r? @locks