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.

Sign up for GitHub

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

Jump to bottom

ID5 userId submodule #3798

Merged
merged 10 commits into from
Jun 24, 2019
Merged

ID5 userId submodule #3798

merged 10 commits into from
Jun 24, 2019

Conversation

padurgeat
Copy link
Contributor

@padurgeat padurgeat commented May 1, 2019 •
edited
Loading

Type of change

  • Feature

Description of change

This PR add a id5 submodule in userId to fetch ID5 user id and add it to the bid request. It is mostly a duplicate of unifiedId code and tests.

Maintainer

padurgeat@id5.io

@bretg bretg requested a review from idettman May 6, 2019 11:42
@bretg
Copy link
Collaborator

bretg commented May 8, 2019

I've requested a partner id for testing by emailing contact@id5.io

We're going to need to update the doc on prebid.org, but that can wait until after inspection

idettman
idettman suggested changes May 8, 2019
View reviewed changes
Copy link
Contributor

@idettman idettman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Everything looks great, with one small change: On line 214, rename GDPRApplies to ...

modules/userId.js Outdated Show resolved Hide resolved
@padurgeat
Copy link
Contributor Author

padurgeat commented May 9, 2019 •
edited
Loading

I've requested a partner id for testing by emailing contact@id5.io

We're going to need to update the doc on prebid.org, but that can wait until after inspection
Hey Bret, you can use the 173

@bretg
Copy link
Collaborator

bretg commented May 13, 2019

@padurgeat - FYI, we're going to be re-factoring the UserId module this week to be more modular. When that's complete, we'd ask that you refactor this Pull Request to fit into the changes. Will post again when the refactoring is complete.

@padurgeat
Copy link
Contributor Author

I'm not really surprised, let me know :)

@padurgeat
Copy link
Contributor Author

@bretg Any news here ? cheers

@bretg
Copy link
Collaborator

bretg commented May 22, 2019

@padurgeat - yes. The refactoring PR has been merged. Please refactor your code to be a 'sub-module' with the name id5IdSystem.js

@padurgeat
Copy link
Contributor Author

@bretg Refactoring done. Let me know if any other change is required. Thanks

@padurgeat
Copy link
Contributor Author

Hey guys, should I push a PR for website documentation, or should we wait to have a least on adapter passing the ID5 id before ? cheers,

@idettman
Copy link
Contributor

idettman commented Jun 6, 2019

@padurgeat Yes, pIease push a PR for website documentation, thanks

idettman
idettman reviewed Jun 6, 2019
View reviewed changes
modules/userId.js Outdated Show resolved Hide resolved
idettman
idettman suggested changes Jun 6, 2019
View reviewed changes
Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

Check my comment for small change request to move the submodule registration out of userId.js and into id5System.js

@idettman
Copy link
Contributor

idettman commented Jun 12, 2019 •
edited
Loading

@padurgeat there’s a circle CI error from duplicates imports, once that’s fixed I’ll approve. Change to:
import {isGDPRApplicable, attachIdSystem} from './userId.js';

@padurgeat
Copy link
Contributor Author

@idettman done!

@idettman
Copy link
Contributor

This is ready to be merged as soon as a docs link is ready

@idettman idettman added the LGTM label Jun 21, 2019
@smenzer smenzer mentioned this pull request Jun 24, 2019
Merged
@smenzer
Copy link
Collaborator

smenzer commented Jun 24, 2019

@idettman docs PR is here: prebid/prebid.github.io#1363

@idettman idettman merged commit de8381f into prebid:master Jun 24, 2019
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
71865ec 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
620a9ee 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
5b8a663 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
c4baf74 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
c4ce4e0 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
d30fc6f 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
59f9735 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
f0f8884 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 2, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
b6d3743 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this pull request Nov 28, 2019
@padurgeat
ID5 userId submodule (prebid#3798)
27e19da 
* Extract GDPRApplies in userId.js to make it available to cookie-sync requiring it

* Adding id5 userId submodule, tests and documentation

* Fixed typo in test name for unifiedid

* Adding test to userId for ID5

* Follow correct naming convention for GDPRApplies: renamed to isGDPRApplicable

* Refactoring of id5 module following refactoring of userId module.

* Moved init of id5 user module at the end of the id5 module itself

* regroup import to avoid a CircleCI Bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idettman idettman idettman approved these changes

Assignees

@idettman idettman

Labels
LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@padurgeat @bretg @idettman @smenzer