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

Update compose pay button to version 0.1.4 #8006

Closed
wants to merge 2 commits into from

Conversation

StefMa
Copy link
Contributor

@StefMa StefMa commented Feb 27, 2024

Summary

Just a small update of the compose pay button dependency.

Motivation

Background is that we are using stripe (so this library 😁 ) together with licensee.
Unfortunately up to version 0.1.3 compose pay button didn't provide a valid <license> tag in their POM.
This prevents us to update stripe as the transitive dependency compose pay button will report as "invalid license" (for us).
This is now fixed, since compose pay button has the default Apache 2 license 🙂

See also the following related issue and PR:
google-pay/compose-pay-button#17
google-pay/compose-pay-button#18

Seems this is the full changelog/diff of the update:
https://github.com/google-pay/compose-pay-button/compare/fc3aacfd0bb632bdda40ba915f616a4a0cd0f87f..3418fd1ffd9dc9d9c88e368e721649f8ad0c62df

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

@StefMa StefMa requested review from a team as code owners February 27, 2024 12:57
@StefMa StefMa requested a review from samer-stripe February 27, 2024 12:57
samer-stripe
samer-stripe previously approved these changes Feb 27, 2024
@StefMa
Copy link
Contributor Author

StefMa commented Mar 2, 2024

@samer-stripe this is approved, but no checks are running... Could you please have a second look here? 🤔

@samer-stripe samer-stripe reopened this Mar 4, 2024
@StefMa
Copy link
Contributor Author

StefMa commented Mar 4, 2024

Hmm.. Seems the annotation is not available anymore 🤔. Maybe even a wrong one was used 🤷‍♂️

Might have a look later...

@StefMa
Copy link
Contributor Author

StefMa commented Mar 5, 2024

@samer-stripe i fixed the annotation.
Seems the wrong one was available (and used) in the classpath that got fixed in the new release 🤷

Now it should work 🙃
At least i can run ./gradlew assembleAndroidTest without issues...

@StefMa
Copy link
Contributor Author

StefMa commented Mar 6, 2024

@samer-stripe whats the problem with the tests? 🤔
I don't get it...

@StefMa
Copy link
Contributor Author

StefMa commented Mar 12, 2024

@samer-stripe ping 🤓

@samer-stripe samer-stripe force-pushed the patch-2 branch 2 times, most recently from 4f32fd1 to f5c4c05 Compare March 12, 2024 19:29
@samer-stripe
Copy link
Collaborator

@StefMa Bitrise and the other GitHub actions won't run on a branch from the remotely forked repo. Could you open a PR from a local fork of this repo?

@StefMa
Copy link
Contributor Author

StefMa commented Mar 12, 2024

Sorry @samer-stripe but I understand what you mean.

What is a local fork? 🤔
Do you mean a git clone? 🤔
Actually I can't push a branch to this repo directly 😁

I already opened a PR a few month ago.
See #7716

Even if that also included failing tests, it seems that @jaynewstrom-stripe somehow got the tests running on at least bitrise... 🤔

@StefMa
Copy link
Contributor Author

StefMa commented Mar 21, 2024

@samer-stripe can we just merge it? 🙈
Or maybe cherry-picking the commits on your behalf and create a PR by yourself? 🤔

@StefMa
Copy link
Contributor Author

StefMa commented Mar 28, 2024

Hey guys,
this PR is a minor dependency bump.
I provided all information you need to evaluate this on your own.
This PR is open for a month already.
Because of our internal security policy we are unable to merge any updates of the stripe-android sdk.
I would really appreciate it if someone could have a look into this.
Again, I'm also fine if someone of you create your own PR. I already suggested it here.
Thank you 🙏

@jaynewstrom-stripe
Copy link
Collaborator

I brought your change in and rebased it here: https://github.com/stripe/stripe-android/pull/8172/files and I also applied our dependency update script, which gives us a detailed view of what changed.

As is, this (transitively) updates compose to 1.6, which we're not quite ready to force all of our downstream consumers onto.

As a workaround, you can depend directly on compose pay button 1.0.0 in your application, which will transitively updated stripes dependency as well, unblocking you from updating in your application.

Thanks for the contribution, once we're ready to update to compose 1.6 and compose pay button 1.0.0, we will take this contribution so you will get attributed.

@StefMa
Copy link
Contributor Author

StefMa commented Apr 4, 2024

Thanks for looking into it 🚀
Probably we will go with the workaround for now.
Thanks!

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 this pull request may close these issues.

3 participants