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

Techdebt/noid/node14 npm7 #5773

Merged
merged 15 commits into from
Jun 17, 2021
Merged

Techdebt/noid/node14 npm7 #5773

merged 15 commits into from
Jun 17, 2021

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Jun 14, 2021

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
@PVince81
Copy link
Member

@nickvergessen thanks for the suggestions. I already changed this in parallel, strange that Github doesn't say anything about this.

Now the errors are about actual code to adjust to satisfy eslint

@PVince81
Copy link
Member

@marcoambrosini the jest stuff is fixed as you asked, do you want to take care of the reported eslint issues ?

@marcoambrosini
Copy link
Member Author

@marcoambrosini the jest stuff is fixed as you asked, do you want to take care of the reported eslint issues ?

Thanks @PVince81, yes I will!

@nickvergessen
Copy link
Member

do you want to take care of the reported eslint issues ?

We talked about it with skjnldsv yesterday and it might also be okay to add a temporary exception to the eslint as it's just a different prefered coding style

@skjnldsv
Copy link
Member

We talked about it with skjnldsv yesterday and it might also be okay to add a temporary exception to the eslint as it's just a different prefered coding style

Yes, requires investigation though, this is an error, not a warning 😁

@marcoambrosini
Copy link
Member Author

Yes, requires investigation though, this is an error, not a warning 😁

Should be a super easy fix, on it

@marcoambrosini
Copy link
Member Author

Ok it should™ all be good now, reviews plz!

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Seems to break calls completely

  • can't see my own video
  • sharing the screen doesn't work anymore
 [Vue warn]: Error in callback for watcher "localMediaModel.attributes.localStream": "TypeError: adapter.browserDetails is undefined"

found in

---> <LocalVideo> at src/components/CallView/shared/LocalVideo.vue
       <Grid> at src/components/CallView/Grid/Grid.vue
         <CallView> at src/components/CallView/CallView.vue
           <MainView> at src/views/MainView.vue
             <AppContent>
               <Content>
                 <App> at src/App.vue
                   <Root> vue.runtime.esm.js:619
TypeError: adapter.browserDetails is undefined
    exports attachmediastream.js:41
    _setLocalStream LocalVideo.vue:261
    localStream LocalVideo.vue:203
    VueJS 5

nickvergessen and others added 14 commits June 17, 2021 16:47
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com>
Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com>
Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com>
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member

Tested with:

  • Edge
  • Safari
  • Firefox
  • Chromium

@marcoambrosini marcoambrosini merged commit 86a5fbf into master Jun 17, 2021
@marcoambrosini marcoambrosini deleted the techdebt/noid/node14-npm7 branch June 17, 2021 15:18

// Toggles videos on and off
handleToggleVideo({ peerId, value }) {
this.sharedDatas[peerId].videoEnabled = value
Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release dependencies Pull requests that update a dependency file technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants