Prebid.js icon indicating copy to clipboard operation
Prebid.js copied to clipboard
verified publisher icon prebid

LuponMedia Bid Adapter: user sync endpoint updated

Open adxpremium opened this issue 3 years ago • 12 comments

Type of change

  • [ x] Code style update (formatting, local variables)

adxpremium avatar Feb 28 '22 21:02 adxpremium

This pull request introduces 1 alert when merging e14acda251f5459d0e8fc40e8ddd58c331e0e605 into 00e0bb59c986faafb5570a920e5ab2ec3e5ea8e5 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Feb 28 '22 22:02 lgtm-com[bot]

Hi @adxpremium , code coverage is at 57.22%, that is below 80%, the required minimum to accept this PR. In order to see code coverage, you can run:

npx gulp test-coverage

followed by

npx gulp view-coverage

musikele avatar Mar 02 '22 10:03 musikele

Hi @adxpremium , code coverage is at 57.22%, that is below 80%, the required minimum to accept this PR. In order to see code coverage, you can run:

npx gulp test-coverage

followed by

npx gulp view-coverage

Hi, we are not sure what code coverage is in this case and what needs to be done on our side to make PR pass the minimum requirements? Could you please explain?

adxpremium avatar Mar 02 '22 14:03 adxpremium

@adxpremium

Hi, we are not sure what code coverage is in this case and what needs to be done on our side to make PR pass the minimum requirements? Could you please explain?

A Prebid rule is that everything that gets merged must have at least 80% code coverage + at least one review.

When we execute unit tests with npx gulp test-coverage, we check that tests are covering at least 80% of total lines of code. If you launch the two commands I wrote above, in your branch, you will be presented with a page with all the code coverage of all modules.

image

musikele avatar Mar 02 '22 14:03 musikele

can you please remove digitrust references?

patmmccann avatar Apr 13 '22 15:04 patmmccann

@patmmccann digitrust is now removed

adxpremium avatar Apr 14 '22 23:04 adxpremium

Tyvm

patmmccann avatar Apr 15 '22 00:04 patmmccann

hi there. I still see tests missing for the following functions:

  • masSizeOrdering
  • setGdprAndPrivacy
  • setUserId
  • setSiteAndUserData
  • setAdserverOrg
  • setLiveIntent
  • setIdentityLink
  • setPubcommon
  • mapSizes
  • parseSizes

Coverage is still stuck at 56.75% , cannot merge.

musikele avatar Apr 20 '22 09:04 musikele

@adxpremium please see https://github.com/prebid/Prebid.js/pull/8372 . Is your use of getLegacyFPD working? It appears not

patmmccann avatar May 06 '22 17:05 patmmccann

This pull request introduces 1 alert when merging 1bdc194a7e153ec3175c0563699ac37214b86c44 into 7a80c65135a5f53592c89ddcd9d57ff2a90c1a81 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar May 12 '22 08:05 lgtm-com[bot]

@adxpremium following up on this pr. Looks like there have been more recent additions here but appears to be a current conflict with master? Were you able to update testing for the above functions?

ChrisHuie avatar Jun 28 '22 14:06 ChrisHuie

@adxpremium review of your adapter cannot proceed until you improve test coverage.

patmmccann avatar Jul 26 '22 13:07 patmmccann

@adxpremium closing this pr for now. Please feel free to reopen or open an issue if you need help with this adapter update.

ChrisHuie avatar Sep 12 '22 16:09 ChrisHuie