CanHub / Android-Image-Cropper

Image Cropping Library for Android, optimised for Camera / Gallery.
Apache License 2.0
1.24k stars 255 forks source link

Stop requiring Personal Access Token #14

Closed manuelrego27 closed 3 years ago

manuelrego27 commented 3 years ago

I suppose it's in the list of planned things to do, but I really don't like including credentials in build.gradle 😅 (or in gradle.properties like you mention in the guide)

EDIT: By this I mean doing whatever changes are needed so a Personal Access Token for GitHub isn't necessary, i've edited the previous title

Canato commented 3 years ago

Hey thanks for the suggestion, this decision was done to make the life easier.

But of course we can improve, if someone add the code and the CI to release it in another place we can change or even have both.

More libraries use the GitHub packages, I believe will became normal to have CIs with access tokens.

Is the same when we have secret values for Twitter API, Facebook API etc We should never commit this values in our code base.

Just to be clear, this library has no access to your access token, only github packages will use it to import the lib.

crocsandcoffee commented 3 years ago

@Canato Why not just publish via JitPack? This way it can be downloaded like any other dependency added to the build.gradle (app)

Canato commented 3 years ago

@Canato Why not just publish via JitPack? This way it can be downloaded like any other dependency added to the build.gradle (app)

Indeed, please could you add the code and if possible the CI for it so we could publish in jitpack?

crocsandcoffee commented 3 years ago

@Canato Sure I can do that, although I am not sure CI setup is really required at this time but FYR (https://jitpack.io/docs/):

Screen Shot 2020-12-20 at 11 28 45 AM

Nothing needs to be safely injected/passed in if the artifact is being published to jitpack. Every time a new version of this library is ready, you just create a new release tag (on master/main) and it will automatically publish the new version to jitpack (after the initial setup).

Then any project that wants to include this library just needs to add the following:

allprojects {
 repositories {
    jcenter()
    ...

    // jitpack.io repository
    maven { url "https://jitpack.io" }

    ...
 }
}

and

implementation 'com.canhub.cropper:android-image-cropper:<tag-version>'

and a badge with the latest release can be added to README like so:

Screen Shot 2020-12-20 at 11 29 17 AM

Also - if you can give me permissions to write to the repo, I can setup jitpack for you

Canato commented 3 years ago

@Canato Sure I can do that, although I am not sure CI setup is really required at this time but FYR (https://jitpack.io/docs/):

Screen Shot 2020-12-20 at 11 28 45 AM

Nothing needs to be safely injected/passed in if the artifact is being published to jitpack. Every time a new version of this library is ready, you just create a new release tag (on master/main) and it will automatically publish the new version to jitpack (after the initial setup).

Then any project that wants to include this library just needs to add the following:

allprojects {
 repositories {
    jcenter()
    ...

    // jitpack.io repository
    maven { url "https://jitpack.io" }

    ...
 }
}

and

implementation 'com.canhub.cropper:android-image-cropper:<tag-version>'

and a badge with the latest release can be added to README like so:

Screen Shot 2020-12-20 at 11 29 17 AM

Perfect! Will be waiting for the PR. Please create a fake release, like 0.0.1 for test the jitpack import before we merge into main

And be sure to connect this issue to the PR

crocsandcoffee commented 3 years ago

@Canato can you add me/give me write permissions so I can make this change? I've already created a branch called jitpack with changes

Canato commented 3 years ago

@crocsandcoffee I'm not close to my laptop now to make it easy. I can do tomorrow. But on the mean time, for test, you can do with the fork version on your personal GitHub.

And tomorrow we do with the "real" code

Canato commented 3 years ago

@crocsandcoffee was able to publish your fork project ?

VEIKKO99 commented 3 years ago

I'm also desperately waiting for this :)

crocsandcoffee commented 3 years ago

Apologies - will have this done later this evening. Will post my updates/changes here but this is an easy task so I'll have it done this evening for sure.

crocsandcoffee commented 3 years ago

Okay - I got it to work on my fork @Canato, I released a new version of the library (1.1.1) via jitpack. If you would like to test to see for yourself, try creating a sample android project (or use an existing one) and do the following:

Add this to your root build.gradle:

allprojects {
   repositories {
    ...
    maven { url 'https://jitpack.io' } // ADD THIS LINE
   }
}

Add this to your app build.gradle:

 implementation 'com.github.crocsandcoffee:Android-Image-Cropper:1.1.1'

Then try referencing any component of the library from your source code and it should import correctly. (Or just simply building the project after adding this dependency should suffice)

crocsandcoffee commented 3 years ago

If everything looks good, let me know, and I can clone this repo and make PR to main with the changes needed for the following:

VEIKKO99 commented 3 years ago

The integration works perfectly now! Thanks!

Btw. modifying of the manifest file is missing from the migration guide. Also on Android 10 I also get the "Failed to load Sampled Bitmap" error but that's a separate issue.

crocsandcoffee commented 3 years ago

@VEIKKO99 keep in mind this is just a test publish version from my fork. I'll have to publish a new version from this repo tomorrow once I get collaborator access, once I do I'll update you so you can include the latest version of this library via jitpack.

We can also update the readme with anything that is missing or needs to be updated based on these changes. 👍

Canato commented 3 years ago

@crocsandcoffee amazing! I invited you to the team and hopefully give the right access. Let me know and let's smash this PR

🚀