-
Notifications
You must be signed in to change notification settings - Fork 231
Release/3.0.0 #480
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
Release/3.0.0 #480
Conversation
…/microsoftgraph/msgraph-sdk-javascript into JS-Migrate-To-ESLint-Part-1_#308
…icrosoftgraph/msgraph-sdk-javascript into nikitha/graph-clientside-error
* Fixing links * Update v3-upgrade-guide.md
* Adding the ChunkRecord interface * Adding more tests for stream resume * comments, assert conditions change * SliceRecord and comments
* 3.0.0 preview 2 package update * restore file
fix docs markdown link
* adding links to samples * removing duplicate links * adding description for typings and removing en-us
* removing node and browser; adding largefile samples * adding links of samples in docs * token credential readme change
* @returns The sliced file part | ||
*/ | ||
public async sliceFile(range: Range): Promise<SliceType> { | ||
let rangeSize = range.maxValue - range.minValue + 1; |
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.
defensive programing on range
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.
Vincent caught some valid issues. Since none of these are release blockers, let's open tracking issues to be addressed in 3.0.1 release which can be done next week. Please open tracking issues and provide links in your response to Vincent.
@MIchaelMainer my main concern here is this
from https://www.npmjs.com/package/msal And I think we should seriously review upgrading to the newer package instead of releasing something with a dependency on a deprecated package. |
@baywet msal-browser authprovider is a feature for 3.1.0. ImplicitMsalAuthenticationProvider for msal 1.0.0 is deprecated so that we give some time for the users to move to the newer auth library. The deprecation of the ImplicitMsalAuthenticationProvider has been mentioned in the code and upcoming blogpost.
|
@nikithauc thanks for the additional information. Is the plan to remove the implicit provider when adding support for msal browser (and removing msal from the deps) ? |
@baywet I will have to check on that and get back. The discussion on when to remove is yet to happen. |
@nikithauc to expand a little bit: removing something, even if it was marked as deprecated before, should be considered a breaking change. In this case, if we decide to keep the implicit flow provider for v3, it means we can only remove it with v4 if we want to avoid breaking people according to semver. This is why I was surprised to see it was still here given the fact the flow has been deprecated for a while now, it forces us to keep a dependency on a deprecated package, and it'll force a new major version. |
@baywet I would like to deprecate this now as discussed and remove it in v4.0. cc: @ddyett |
@baywet I have updated the node requirement as I felt that would be relevant for this release. Thanks for pointing that out! The other enhancement changes will be addressed in other PRs. |
#444
Changes in this release are added in the 3.0.0 Milestone and
the V3 upgrade guide
cc: @roinochieng