Skip to content

Conversation

@julien-nc
Copy link
Member

@julien-nc julien-nc commented Sep 1, 2020

Matterbridge process state is updated and displayed when saving bridge config and when showing log file content.

Seeing log file content is mandatory when setting up Ms-teams as Matterbridge provides a link to finalize sign-in in those logs.

Not sure about the "show mattberbridge log" button text and location.

Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc julien-nc added 3. to review enhancement feature: integration 📦 Integration with 3rd party (chat) service labels Sep 1, 2020
@nickvergessen nickvergessen added this to the 💚 Next Major (20) milestone Sep 2, 2020
@julien-nc
Copy link
Member Author

Here is a clarified version. Like you said, it's better if editBridge returns the state.

So we now get process state every minute. The loop is relaunched when changing the room. We could use a much lower frequency. What do you think?

Comment on lines +80 to +81
$result = $this->bridgeManager->getBridgeProcessState($this->room);
return new DataResponse($result);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$result = $this->bridgeManager->getBridgeProcessState($this->room);
return new DataResponse($result);
$state = $this->bridgeManager->getBridgeProcessState($this->room);
return new DataResponse($state);

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.

LGTM but didn't execute

@nickvergessen nickvergessen merged commit 5ceeacd into master Sep 3, 2020
@nickvergessen nickvergessen deleted the bridge-process-state branch September 3, 2020 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review enhancement feature: integration 📦 Integration with 3rd party (chat) service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants