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

YieldNexus: Rename adapter's name #3508

Merged
merged 7 commits into from
Feb 6, 2019

Conversation

sa1omon
Copy link
Contributor

@sa1omon sa1omon commented Feb 4, 2019

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

Renamed yieldNexusBidAdapter.js to yieldnexusBidAdapter.js in order to fix download issue.

@bretg
Copy link
Collaborator

bretg commented Feb 4, 2019

@sa1omon - you'll need to rename the dev-docs/bidders/yieldNexus.md file over in https://github.com/prebid/prebid.github.io as well

Note that renaming like this is likely to cause issues with the Download page -- I believe users will get errors when trying to retrieve yieldnexus for previous releases.

Assuming you're ok with this, I'll merge after the docs PR is submitted.

@bretg bretg self-assigned this Feb 4, 2019
@sa1omon
Copy link
Contributor Author

sa1omon commented Feb 4, 2019

@sa1omon - you'll need to rename the dev-docs/bidders/yieldNexus.md file over in https://github.com/prebid/prebid.github.io as well

Note that renaming like this is likely to cause issues with the Download page -- I believe users will get errors when trying to retrieve yieldnexus for previous releases.

Assuming you're ok with this, I'll merge after the docs PR is submitted.

Hi @bretg,

  • The reason for the renaming is because a few days a go I have tried to download the yieldnexus bidder adapter in the prebid.github.io download page, but I got an error.
    Do you know why it is happening? Didn't happen before...
    I assumed that this issue is a result of a mismatch between the bidder code name in prebid.github.io (dev-docs/bidders folder) repository and the name of the adapter in the Prebid.js (modules folder) repository.
    But since I didn't change this file in the last PR, I assumed that maybe the reason is something else.
  • From my investigation, the bidder code in the check boxes in the download page is based on the bidder code entry value inside the yieldNexus.md file, and not on the name of the file itself.
    Please let me know if I'm wrong.
    Thanks.

@bretg
Copy link
Collaborator

bretg commented Feb 6, 2019

@sa1omon - I changed the biddercode in the docs .md file and yieldNexus now downloads.

Can we close this PR or would you prefer to make the name change anyhow?

@sa1omon
Copy link
Contributor Author

sa1omon commented Feb 6, 2019

@sa1omon - I changed the biddercode in the docs .md file and yieldNexus now downloads.

Can we close this PR or would you prefer to make the name change anyhow?

Hi @bretg,
I like that the download is working, but after some checks against our code i noticed that prebid.js doesn’t find the bidder with the camel case version (yieldNexus).
This means that the current documentation is misleading and will not work.
How can we keep the lower case bidder code name (in the doc and the code ) and fix the download page?

Thanks

@bretg
Copy link
Collaborator

bretg commented Feb 6, 2019

Sorry - what's the problem? yieldNexus is camel case almost everywhere right now and downloads.

The only place it's not camel case is in https://github.com/prebid/Prebid.js/blob/master/modules/yieldNexusBidAdapter.js

code: 'yieldnexus',

If what you're saying is that bids don't actually work, that's likely the problem. So again - if you want to rename everything to lower case, that's fine. Or you can just fix yieldNexusBidAdapter.js to be camel case and all should be good.

@sa1omon
Copy link
Contributor Author

sa1omon commented Feb 6, 2019

Sorry - what's the problem? yieldNexus is camel case almost everywhere right now and downloads.

The only place it's not camel case is in https://github.com/prebid/Prebid.js/blob/master/modules/yieldNexusBidAdapter.js

code: 'yieldnexus',

If what you're saying is that bids don't actually work, that's likely the problem. So again - if you want to rename everything to lower case, that's fine. Or you can just fix yieldNexusBidAdapter.js to be camel case and all should be good.

Hi @bretg,
Yes, the problem is that the bids don't actually work.
From that reason I prefer to rename everything to lower case.
Thanks.

@bretg bretg removed the question label Feb 6, 2019
@bretg
Copy link
Collaborator

bretg commented Feb 6, 2019

Ok.

@bretg bretg merged commit 1b41d63 into prebid:master Feb 6, 2019
@bretg bretg removed the needs docs label Feb 6, 2019
jacekburys-quantcast pushed a commit to jacekburys-quantcast/Prebid.js that referenced this pull request May 15, 2019
* Add support for multiple media types. Add test coverage.

* Add support for multiple media types. Add test coverage.

* Modify multi-format ads handler. Modify tests.

* Rename yieldnexus bid adapter to fix download issue
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