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

Why Administrator do not merge PR in the recent month? #715

Open
canven opened this issue Oct 30, 2020 · 22 comments
Open

Why Administrator do not merge PR in the recent month? #715

canven opened this issue Oct 30, 2020 · 22 comments

Comments

@canven
Copy link

canven commented Oct 30, 2020

I find lots of PR which is useful, but it seems no person to handle it?
Have anybody can help to merge PR as below?

feat(i18n): update Simplified and traditional Chinese language #533

@gianpaolodn
Copy link

The repo seems abandoned to me

@fabio-blanco
Copy link

The repo seems abandoned to me

I had the same perception. I will probably start a forked project based on this. Just waiting to see if I get some interaction.

@mikehardy
Copy link

mikehardy commented Mar 6, 2021

This fork seems to have the most recent activity based on the github dependency graph - hey @simpson-keyvalue - this is easily the biggest crop library on github by community size, and has other ecosystems (like react-native's crop) that ride on it, making it a very large audience. You seem like the most active current coder. Perhaps a call for contributors and sharing access on your repo along with a roadmap and you're in business with lots of helpers 🤔 ?

@simpson-keyvalue
Copy link

hey @mikehardy , I had a requirement for one of my projects, which is why I forked and worked on it separately. My modification was to make this capable of processing multiple images.

@mikehardy
Copy link

Ah ha :-) - probably not exactly on target to host the whole community then ;-). @fabio-blanco you might be it! All the fame and glory and internet points could be yours

@fabio-blanco
Copy link

fabio-blanco commented Mar 8, 2021

Ah ha :-) - probably not exactly on target to host the whole community then ;-). @fabio-blanco you might be it! All the fame and glory and internet points could be yours

Yes, as I've stated before, I will start a forked project based on this. I'm working on a personal project at the present moment and this is taking all my time in this days but as soon as I can I intend to change some things on the project to make clear it is a forked project, maybe do some improvements in one or other thing too and I will see if it is possible to do what I've done in the #732 pool request but in the native branch in order to save it. Until I do this, maybe the Yalantis folks may decide to not let this repository die. If they don't then I will go on.
But I will not do this for fame nor glory, I will do this just to keep the project alive. I have plans on using this lib on my own personal projects too.

@mikehardy
Copy link

I was joking about the fame/glory of course :-) - I just incorporated this in https://github.com/ankidroid/Anki-Android/ (open source app with nearly 2MM daily users and donations to fund development) so I'll have some time to help for sure, though I won't have time to manage the community

And of course if the Yalantis folks want to keep it alive (perhaps by sharing out the power somewhat, allowing others to be able to review/merge/release?) I'm always happy to collaborate personally. Forks are a last resort but if there is no activity here we won't be able to avoid it

@fabio-blanco
Copy link

I was joking about the fame/glory of course :-)

Yeah, I know it... And I also know that it has a lot more of work than fame and glory :)

I just incorporated this in https://github.com/ankidroid/Anki-Android/ (open source app with nearly 2MM daily users and donations to fund development) so I'll have some time to help for sure, though I won't have time to manage the community

Nice project. Time is indeed a scarce resource. I'm always dealing with it.

And of course if the Yalantis folks want to keep it alive (perhaps by sharing out the power somewhat, allowing others to be able to review/merge/release?) I'm always happy to collaborate personally. Forks are a last resort but if there is no activity here we won't be able to avoid it

I agree.

@vcuk21
Copy link

vcuk21 commented Mar 16, 2021

I maybe can make a suggestion... not based on this library, but fix the issue @fabio-blanco did here #732 (when OS 10/11)

There is this new repo. Is a Handover from ArthurHub Image Crop
https://github.com/CanHub/Android-Image-Cropper

ArthurHub had 5k stars, this new CanHub lib had 1k downloads in the begin of the year and now has 4k, so look like is growing.

I took the time and changed from uCrop to this one.
Still missing some features but is working in all OSs and I can see people activily working on this.

For sure we can drop some PRs there.

@mikehardy
Copy link

@vcuk21 from that library:

Step 3. Add permissions to manifest

<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>

That is not okay in 2021

This library actually works (without needing external storage unless I am missing something) for the current Android levels and looks pretty good. Just needs a home.

@Canato
Copy link

Canato commented Mar 17, 2021

@vcuk21 from that library:

Step 3. Add permissions to manifest

<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>

That is not okay in 2021

This library actually works (without needing external storage unless I am missing something) for the current Android levels and looks pretty good. Just needs a home.

I can answer this @mikehardy

We make the decision of keep the permission for devices on OS 9 or under.

This is because there is a know bugs with some devices in some OS, mostly common in Huawei phones when we get the scope URI.

Só to avoid this cases in previous OS we keep the old external storage permission working.

This bought more stability with the permissions and scope storage changes.

But you are right we don't need for OS 10 or 11. I will update the documentation soon, to add "maxSDK" parameter to this permissions

@mikehardy
Copy link

@Canato interesting! I'm familiar with that believe it or not 😅 - I'm still not sure you need external storage, you can internalize files to cache. You might like https://github.com/ankidroid/Anki-Android/pull/6543/files#diff-ed0d7051166502bcfca45541b88fdec2d4d86fb7c385a55bb6663274543d7efaR649-R755

@Canato
Copy link

Canato commented Mar 17, 2021

@Canato interesting! I'm familiar with that believe it or not 😅 - I'm still not sure you need external storage, you can internalize files to cache. You might like https://github.com/ankidroid/Anki-Android/pull/6543/files#diff-ed0d7051166502bcfca45541b88fdec2d4d86fb7c385a55bb6663274543d7efaR649-R755

Indeed a Android issue, cause when we use Environment.DIRECTORY_PICTURES and this return a path that should be default, but some companies changed this for any reason, breaking the code.

I did a high level check on this Merged PR, I'm in holidays and without laptop until monday, so trying to hold myself hehehe. Still I need to understand better how you achieve the issue with some label URI path problems.
I saw this code:

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
    uri = FileProvider.getUriForFile(mActivity, mActivity.getApplicationContext().getPackageName() + ".apkgfileprovider", file);
} else {
    uri = Uri.fromFile(file);
}

And I tried it before, but keep the issue in some cases.

And with external-cache-path would we need some cache cleaning after some time?

In the end I got this hack and not optimal code https://github.com/CanHub/Android-Image-Cropper/blob/main/cropper/src/main/java/com/canhub/cropper/utils/GetUriForFile.kt
Is ugly and I want to revisit, but was tested on 4M devices on production and was working in all OSs from 5 to 11 and diverse labels, from Huawei, Pixel to Samsungs etc

@mikehardy
Copy link

Enjoy your holidays! None of this is a rush at all :-)

It may be that we (AnkiDroid) still have issues on certain devices. We haven't had reports of any recently but we certainly were not cropping with Android 11, thus my interest in uCrop (which we integrated for Android 11 only to this point) and thus maybe Android-Image-Cropper

Cheers

@Altonss
Copy link

Altonss commented Nov 29, 2021

I found a bug with the tool incorporated in the app Catima with Android 5.0.1 CatimaLoyalty/Android#636.
So is this library still developed or has a fork been continued?

@mikehardy
Copy link

No merge here since August 2020. https://github.com/CanHub/Android-Image-Cropper had a merge 14 days ago.

@Altonss
Copy link

Altonss commented Nov 29, 2021

No merge here since August 2020. https://github.com/CanHub/Android-Image-Cropper had a merge 14 days ago.

Thanks for the recommendation! Are you using this library on AnkiDroid and does it fit your needs?

@fabio-blanco
Copy link

fabio-blanco commented Nov 29, 2021

No merge here since August 2020.

That's not true. My PR #732 was merged on 25/05/2021. But this was only for the non native version.
Yet things here seems to be a little bit stagnated.

@Altonss
Copy link

Altonss commented Nov 29, 2021

Ah yes sorry I didn't see that. What is the difference between the branches?

@fabio-blanco
Copy link

Ah yes sorry I didn't see that. What is the difference between the branches?

  • master => Native version (uses C++ code for improved performance)
  • master-non-native => Non native version (java only)

It means that the latest version has Android 10+ support but has no native version available.
Unfortunately I had no time to research more on the native version in order to implement functionalities equivalent from what I did at the #732 PR.

@mikehardy
Copy link

No merge here since August 2020. https://github.com/CanHub/Android-Image-Cropper had a merge 14 days ago.

Thanks for the recommendation! Are you using this library on AnkiDroid and does it fit your needs?

yes and yes

@Canato
Copy link

Canato commented Nov 30, 2021

Version 4.0.0 released just now, many changes, need good tests XD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants