Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@zhixzhan
Copy link
Contributor

@zhixzhan zhixzhan commented Apr 2, 2021

Description

Task Item

#minor

Screenshots

@srinaath srinaath self-assigned this Apr 2, 2021
@srinaath
Copy link
Contributor

srinaath commented Apr 2, 2021

@zhixzhan  lets wait on @garypretty  to review this content before we merge. So Tuesday is probably when we are looking to land this.

boydc2014
boydc2014 previously approved these changes Apr 2, 2021
@boydc2014
Copy link
Contributor

@zhixzhan  lets wait on @garypretty  to review this content before we merge. So Tuesday is probably when we are looking to land this.

Sounds good, i think it is provided by @garypretty , do you want to confirm?

@cwhitten
Copy link
Member

cwhitten commented Apr 5, 2021

I'd like to understand the following before we take this:

  1. Is this feed fetched every time the home page loads? If so that seems inefficient.
  2. Do we know what type of latency we can anticipate when we load this from github? Have we considered publishing the resource to a CDN?
  3. What error cases are considered in the implementation? Malformed data, network errors, etc.

@boydc2014
Copy link
Contributor

I'd like to understand the following before we take this:

  1. Is this feed fetched every time the home page loads? If so that seems inefficient.
  2. Do we know what type of latency we can anticipate when we load this from github? Have we considered publishing the resource to a CDN?
  3. What error cases are considered in the implementation? Malformed data, network errors, etc.

For 1, i think the current implementation loads it every time Composer loads, not every time home page load. Is this correct @zhixzhan?
For 2, we haven't measured much about the latency, manual testing of the home page PR shows it's quite good, it could be there is less load. srinaath helped investigated the CDN usage, the recommendation is it's a bit of over kill given the effort to onboard, @srinaath can you elaborate more on this?
For 3, the feed defines a certain widgets (news, docs) in the home pages, when any error happens, the widget will be empty. The core functionality like creating\open, will still be functional.

@zhixzhan
Copy link
Contributor Author

zhixzhan commented Apr 6, 2021

  1. Is this feed fetched every time the home page loads? If so that seems inefficient.

Hi @cwhitten @boydc2014 the feed will be fetched at every composer loads.

@coveralls
Copy link

coveralls commented Apr 6, 2021

Coverage Status

Coverage remained the same at 51.691% when pulling 3ff7c6f on zhixzhan/feed into aa18b9e on main.

@boydc2014 boydc2014 merged commit 1bff121 into main Apr 7, 2021
@boydc2014 boydc2014 deleted the zhixzhan/feed branch April 7, 2021 00:38
@cwhitten cwhitten mentioned this pull request May 20, 2021
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* add feed

* update for test

Co-authored-by: Dong Lei <donglei@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants