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

Pass currency converter to modules #3278

Merged

Conversation

pm-nilesh-chate
Copy link
Contributor

No description provided.

@hhhjort
Copy link
Collaborator

hhhjort commented Nov 7, 2023

What you have looks good. Seems that you still have to clean up some old tests to reflect your changes, and should have some new tests to make sure the new features work properly.

@pm-nilesh-chate pm-nilesh-chate marked this pull request as ready for review November 9, 2023 10:30
@pm-nilesh-chate
Copy link
Contributor Author

refactored the current unit tests to cover all the required scenarios. Please review :)

customRates pbsRates UsePBSRates conversion
VALID VALID FALSE customRates
VALID VALID TRUE customRates+pbsRates
VALID NIL FALSE customRates
VALID NIL TRUE customRates
EMPTY VALID FALSE ERROR
NIL VALID TRUE pbsRates
EMPTY NIL FALSE ERROR
EMPTY NIL TRUE ERROR
NIL NIL NA ERROR
EMPTY VALID TRUE pbsRates

Copy link
Collaborator

@hhhjort hhhjort left a comment

Choose a reason for hiding this comment

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

LGTM

@bsardo bsardo changed the title pass currency converter to modules Pass currency converter to modules Nov 16, 2023
@bsardo bsardo merged commit e5c920a into prebid:master Nov 20, 2023
3 checks passed
@pm-nilesh-chate pm-nilesh-chate deleted the pass-currency-converter-to-modules-1 branch November 21, 2023 04:56
svamiftah pushed a commit to sovrn/prebid-server that referenced this pull request Nov 21, 2023
SuprPhatAnon pushed a commit to GiftConnect/prebid-server that referenced this pull request Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants