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

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

wants to merge 1 commit into from

Conversation

bretg
Copy link
Contributor

@bretg bretg commented Sep 6, 2024

@bsardo to enhance this checklist

@bretg bretg requested a review from bsardo September 6, 2024 14:29
@bsardo bsardo self-assigned this Sep 9, 2024
@@ -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.

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.

3 participants