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

Dependencies: resolve maven conflicts between core and bundle #3793

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

osulzhenko
Copy link
Collaborator

@osulzhenko osulzhenko commented Feb 27, 2025

🔧 Type of changes

  • new bid adapter
  • bid adapter update
  • new feature
  • new analytics adapter
  • new module
  • module update
  • bugfix
  • documentation
  • configuration
  • dependency update
  • tech debt (test coverage, refactorings, etc.)

✨ What's the context?

What's the context for the changes?

🧠 Rationale behind the change

Why did you choose to make these changes? Were there any trade-offs you had to consider?

🔎 New Bid Adapter Checklist

  • verify email contact works
  • NO fully dynamic hostnames
  • geographic host parameters are NOT required
  • direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
  • if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
  • cover an adapter configuration with an integration test

🧪 Test plan

How do you know the changes are safe to ship to production?

🏎 Quality check

  • Are your changes following our code style guidelines?
  • Are there any breaking changes in your code?
  • Does your test coverage exceed 90%?
  • Are there any erroneous console logs, debuggers or leftover code in your changes?

@osulzhenko osulzhenko added the dependencies Pull requests that update a dependency file label Feb 27, 2025
@osulzhenko osulzhenko self-assigned this Feb 27, 2025
@osulzhenko
Copy link
Collaborator Author

The google-cloud-storage dependency creates conflicts on Maven dependencies, causing a mismatch between the core and bundle versions in all builds. @EvgeniiMunin will this change be okay for you?
Screenshot from 2025-02-27 10-41-25
Screenshot from 2025-02-27 10-37-11

@EvgeniiMunin
Copy link
Contributor

EvgeniiMunin commented Feb 27, 2025

Excluding failureaccess should be ok if it resolves the conflict on maven build and does not break the GCS fetching

<exclusions>
<exclusion>
<artifactId>failureaccess</artifactId>
<groupId>com.google.guava</groupId>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please swap groupId and artifactId. There is an unspoken convention that groupId should go before artifactId.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And while we're at it, it might be worth bumping dependencies for this module. Google cloud is being updated quite frequently, and we're already several versions behind.

@osulzhenko osulzhenko force-pushed the resolve-maven-conflicts branch from 3ef0ac3 to f3a843c Compare March 4, 2025 14:20
@osulzhenko osulzhenko force-pushed the resolve-maven-conflicts branch from ffab070 to 367618a Compare March 7, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants