-
Notifications
You must be signed in to change notification settings - Fork 440
TEZ-4411: Update FileSaver dependency #206
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
|
Solution Credits: @abstractdog Please help with the review and merge as this is a blocker for other commits. |
|
💔 -1 overall
This message was automatically generated. |
|
thanks @guptanikhil007 |
|
@abstractdog There may be an issue with pre-commit pipeline. |
|
There is a failure on master but patch is passing: |
|
@guptanikhil007 : I see, that's kind of my concern, as we want to fix precommit here too (not only local builds), I want to make sure that once we merge this change, precommit will work I mean, what you attached: ...exactly belongs to the log file I mentioned above: precommit is supposed to pick up the source code changes in the pull request, I'm just looking for the reason why it failed, and tried to fetch Teleborder version |
@abstractdog Build Steps: |
| "0.1.1": "7a13f6dba5a340e1cb9e0b64c1c711e4d7edaca1" | ||
| }, | ||
| "https://github.com/eligrey/FileSaver.js.git": { | ||
| "1.3.4": "b4a918669accb81f184c610d741a4a8e1306aa27" |
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.
I think it would make sense to point to same commit (eligrey/FileSaver.js@b7cf622) in this PR so there are no extra changes.
Upgrading can be handled in separate commit.
|
thanks @guptanikhil007, makes sense, so it should work after merging |
|
The lowest bower version available is 1.2.0. @Deependra-Patel @abstractdog Should we go with 1.2.0? |
I cannot tell honestly, I've just uploaded the one on my laptop to: https://issues.apache.org/jira/secure/attachment/13043100/FileSaver.js instead of manually looking for changes, please do the following: if all tests pass, I'm fine with the version, no matter which 1.x |
|
mvn test is passing with 1.2.0 |
|
thanks @guptanikhil007 , I'm about to merge this soon |
|
💔 -1 overall
This message was automatically generated. |
|
merged, created empty PR to check mvninstall phase: #207 |
What I meant was we could have kept the commit id |
…y Laszlo Bodor, Deependra Patel)
…ewed by Laszlo Bodor, Deependra Patel)
What changes were proposed in this pull request?
Update FileSaver dependency to latest bower supported dependency
Why are the changes needed?
Previous dependency files were no longer available
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Manual tests.