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

fix: removed materialshowcase libary due to crash on android 10 #2934

Merged
merged 1 commit into from
Nov 24, 2019

Conversation

codedsun
Copy link
Contributor

Fixed #2930

Changes: Removed material showcase library

Screenshots of the change:

@auto-label auto-label bot added the fix label Nov 23, 2019
@codedsun
Copy link
Contributor Author

@iamareebjamal @yashk2000 Please check and merge!

yashk2000
yashk2000 previously approved these changes Nov 24, 2019
@codedsun
Copy link
Contributor Author

@iamareebjamal Please get this done

@iamareebjamal
Copy link
Member

Should have replaced with some other library

@codedsun
Copy link
Contributor Author

@iamareebjamal Shall I do this in this PR or another issue?

@iamareebjamal
Copy link
Member

Why would I merge a PR on which I have requested changes and which would leave the app in a broken state?

@codedsun
Copy link
Contributor Author

Why should this leave the app in broken state? It has removed the library but not replaced with some other, but didn't broke anything. @iamareebjamal

@iamareebjamal
Copy link
Member

That's called broken. Every commit of the repo should have correct build of app. This is clearly incomplete work, so should not be merged as a commit, regardless of if it breaks something or not.

If you just add a blank activity and say nothing is broken, and I'll add UI in another PR and logic in yet another PR, none of those PRs should be merged because the feature is incomplete and if the user will compile the app from a particular commit, he/she'll get an incomplete app, doesn't matter if the app crashes or not

@codedsun
Copy link
Contributor Author

@iamareebjamal Okay sending the PR asap

@codedsun
Copy link
Contributor Author

https://gifyu.com/image/vcHx

@iamareebjamal Check this now

@iamareebjamal iamareebjamal merged commit 8cd3a55 into fossasia:development Nov 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phimp.me app crashed on opeing. Android 8.1.0 #2930
3 participants