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

Stop requiring Personal Access Token #14

Closed
manuelrego27 opened this issue Dec 16, 2020 · 15 comments · Fixed by #22
Closed

Stop requiring Personal Access Token #14

manuelrego27 opened this issue Dec 16, 2020 · 15 comments · Fixed by #22

Comments

@manuelrego27
Copy link
Contributor

manuelrego27 commented Dec 16, 2020

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

@manuelrego27 manuelrego27 changed the title Move Library to Maven Stop requiring Personal Access Token Dec 16, 2020
@Canato
Copy link
Member

Canato commented Dec 16, 2020

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.

@Canato Canato added this to the project_settings milestone Dec 16, 2020
@crocsandcoffee
Copy link
Contributor

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

@Canato
Copy link
Member

Canato commented Dec 20, 2020

@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
Copy link
Contributor

crocsandcoffee commented Dec 20, 2020

@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
Copy link
Member

Canato commented Dec 20, 2020

@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
Copy link
Contributor

@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
Copy link
Member

Canato commented Dec 20, 2020

@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
Copy link
Member

Canato commented Dec 21, 2020

@crocsandcoffee was able to publish your fork project ?

@VEIKKO99
Copy link

I'm also desperately waiting for this :)

@crocsandcoffee
Copy link
Contributor

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
Copy link
Contributor

crocsandcoffee commented Dec 23, 2020

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
Copy link
Contributor

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:

  • configuration changes needed for jitpack
  • new release version via jitpack (1.11)
  • the jitpack release version badge added to README
  • updates to the README for how this library can be included

@VEIKKO99
Copy link

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
Copy link
Contributor

crocsandcoffee commented Dec 23, 2020

@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
Copy link
Member

Canato commented Dec 23, 2020

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

🚀

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

Successfully merging a pull request may close this issue.

4 participants