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

Update code-reviews.md #3905

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update code-reviews.md #3905

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/developers/code-reviews.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ Some examples include:
- Does the code use any global, mutable state? [Inject dependencies](https://en.wikipedia.org/wiki/Dependency_injection) instead!
- Can the code be organized into smaller, more modular pieces?
- Is there dead code which can be deleted? Or TODO comments which should be resolved?
- Look for code used by other adapters. Encourage adapter submitter to utilize common code.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is you intent with adding this? This could be interpreted as preferring adapters to import / reference other adapters, but that's not desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SyntaxNode - there's been a major push in the Prebid.js world to completely eliminate code duplication. They run an automated scanner that alerts of any copy-paste activity.

I don't think PBS needs such a hard stance, but obvious copy-paste of whole adapters should be noticed and discouraged. There are several options: either they could utilize aliases, or pull common code out into a library. e.g. on the PBJS side, there are now several adapter libraries. The issue is that there are some 'white labeling' outfits that crank out bid adapters for other companies, often doing 80% of the work with copy-paste. So PBJS has encouraged those companies to up their game with common code libraries, which has cut the amount of adapter code signficantly.

Loading