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

Rule enforcement: support for floors must include the floors module #6804

Merged
merged 146 commits into from
Jun 8, 2021

Conversation

patmmccann
Copy link
Collaborator

Type of change

  • Other

Description of change

Enforcement of #6465 for #5966 release

I have tried to remove floor support for recently maintained adapters when possible, otherwise adapters were removed.

@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2021

This pull request fixes 3 alerts when merging fde4b7f into d34bd45 - view on LGTM.com

fixed alerts:

  • 2 for Useless conditional
  • 1 for Useless assignment to local variable

@ChrisHuie ChrisHuie self-requested a review June 7, 2021 17:24
@lgtm-com
Copy link

lgtm-com bot commented Jun 7, 2021

This pull request fixes 3 alerts when merging 45eab11 into 61c494a - view on LGTM.com

fixed alerts:

  • 2 for Useless conditional
  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Jun 8, 2021

This pull request fixes 3 alerts when merging ce1f527 into c8058a6 - view on LGTM.com

fixed alerts:

  • 2 for Useless conditional
  • 1 for Useless assignment to local variable

@robertrmartinez
Copy link
Collaborator

robertrmartinez commented Jun 8, 2021

What happened to this file

test/spec/modules/smilewantedBidAdapter_spec.js

Looks like it was renamed to

test/spec/smilewantedBidAdapter_spec.js

Which is not correct I believe.

Very last change of files when I look at it:

image

@patmmccann
Copy link
Collaborator Author

What happened to this file

test/spec/modules/smilewantedBidAdapter_spec.js

Looks like it was renamed to

test/spec/smilewantedBidAdapter_spec.js

Which is not correct I believe.

Very last change of files when I look at it:

image

fixed ty

@robertrmartinez
Copy link
Collaborator

I counted

  • 29 modules removed
  • 26 Spec files removed

I may have missed something but should this be equal number?

@patmmccann
Copy link
Collaborator Author

Not every module had a spec file

@robertrmartinez
Copy link
Collaborator

Also I just realized I double counted some because the .md file!

@robertrmartinez
Copy link
Collaborator

Looks good.

The changes where we removed the ability to pass a floor, but just set it to default is what we want for sure right?

Seems like the right thing imo.

Thanks this was a lot of work I am sure!

@lgtm-com
Copy link

lgtm-com bot commented Jun 8, 2021

This pull request fixes 3 alerts when merging 2eb6a4c into c8058a6 - view on LGTM.com

fixed alerts:

  • 2 for Useless conditional
  • 1 for Useless assignment to local variable

@patmmccann
Copy link
Collaborator Author

patmmccann commented Jun 8, 2021

We're hoping those adapters, eg spotx, add support shortly. The only other option I can think of is removing them, so I tried for any adapter that seemed recently maintained to set the default. The deletions are almost all not maintained. The couple exceptions to that are okay with deprecations (eg sortable).

@patmmccann patmmccann merged commit 1ac7c3b into prebid-5.0 Jun 8, 2021
@patmmccann patmmccann deleted the enforce-floors branch June 8, 2021 20:56
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.

4 participants