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

Adding new vendor IncrementX #39237

Merged
merged 15 commits into from
Jul 25, 2023
Merged

Adding new vendor IncrementX #39237

merged 15 commits into from
Jul 25, 2023

Conversation

Prebid-Vertoz
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 10, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@amp-owners-bot amp-owners-bot bot requested a review from banaag July 10, 2023 10:27
@amp-owners-bot
Copy link

amp-owners-bot bot commented Jul 10, 2023 •
edited by ampprojectbot
Loading

Hey @ampproject/wg-caching! These files were changed:

validator/testdata/feature_tests/ads.html

@erwinmombay erwinmombay requested review from powerivq and removed request for banaag July 12, 2023 17:43
powerivq
powerivq approved these changes Jul 12, 2023
View reviewed changes
3p/integration.js Outdated Show resolved Hide resolved
@Prebid-Vertoz
Update integration.js
6e3328f 
removed the lines of incrementx
@powerivq
Copy link
Contributor

Still need to fix the check failures. Plz look at the log. Should be pretty easy fix.

@Prebid-Vertoz
Update ads.out
1534db0 
removed incrementx lines
@Prebid-Vertoz
Copy link
Contributor Author

Prebid-Vertoz commented Jul 14, 2023 •
edited
Loading

Still need to fix the check failures. Plz look at the log. Should be pretty easy fix.

We have removed the lines incrementx from 3p/integration.js and ads.out files can you please check and let us know if the commits are correct ?

@powerivq
Copy link
Contributor

powerivq commented Jul 14, 2023 •
edited
Loading

@Prebid-Vertoz It is still failing here: https://app.circleci.com/pipelines/github/ampproject/amphtml/28467/workflows/b08c51be-f7ff-4896-88c5-0e38a5ca5cc8/jobs/604813

Hint: You need to remove the trailing whitespace.

@powerivq
Copy link
Contributor

Also you need to sign the CLA. You can follow the button in the comment above,.

@Prebid-Vertoz
Update ads.amp.esm.html
cdd1460 
removed whitespaces on both the lines
@Prebid-Vertoz
Copy link
Contributor Author

@Prebid-Vertoz It is still failing here: https://app.circleci.com/pipelines/github/ampproject/amphtml/28467/workflows/b08c51be-f7ff-4896-88c5-0e38a5ca5cc8/jobs/604813

Hint: You need to remove the trailing whitespace.

Remove the whitespaces and also sign the CLA

@powerivq
Copy link
Contributor

Please fix: https://app.circleci.com/pipelines/github/ampproject/amphtml/28498/workflows/2241aba2-209b-44f3-bb5f-341351f90153/jobs/605622

You can run amp lint locally to ensure you fix all lint issues.

@Prebid-Vertoz
Copy link
Contributor Author

Please fix: https://app.circleci.com/pipelines/github/ampproject/amphtml/28498/workflows/2241aba2-209b-44f3-bb5f-341351f90153/jobs/605622

You can run amp lint locally to ensure you fix all lint issues.

We fixed all lint issues

@powerivq powerivq requested a review from banaag July 18, 2023 17:30
@powerivq
Copy link
Contributor

Added @banaag for validator approval.

@Prebid-Vertoz
Copy link
Contributor Author

Added @banaag for validator approval.

Hello,
Any update?

@powerivq powerivq merged commit 2b25653 into ampproject:main Jul 25, 2023
10 checks passed
danielrozenberg added a commit to danielrozenberg/amphtml that referenced this pull request Jul 26, 2023
@danielrozenberg
Fix validator tests broken by ampproject#39237
c8c4919
danielrozenberg added a commit that referenced this pull request Jul 26, 2023
@danielrozenberg
✅ Fix validator tests broken by #39237 (#39298)
62e5b43
@powerivq powerivq mentioned this pull request Aug 2, 2023
Closed
banaag added a commit to banaag/amphtml that referenced this pull request Sep 1, 2023
@banaag
cl/552881955 Two-way sync for PR ampproject#39237. No-op, or fixes me…
0f0a769 
…rge conflicts, if any.
@banaag banaag mentioned this pull request Sep 1, 2023
Merged
banaag added a commit that referenced this pull request Sep 1, 2023
@banaag
Validator rollup (#39427)
49ee0db 
* cl/552881955 Two-way sync for PR #39237. No-op, or fixes merge conflicts, if any.

* cl/555542556 n/a

* Updates ads.out to make validator-tests.js pass

* Update some files that fail lint.
eszponder pushed a commit to krzysztofequativ/amphtml that referenced this pull request Apr 22, 2024
@Prebid-Vertoz @eszponder
Adding new vendor IncrementX (ampproject#39237)
fd0a9d1 
* Add files via upload

* Add files via upload

* Add files via upload

* Update integration.js

* Update _config.js

* Update ads.amp.html

* Update ads.amp.html

* Update ads.amp.esm.html

* Update amp-ad.md

* Update ads.html

* Update ads.out

* Update integration.js

removed the lines of incrementx

* Update ads.out

removed incrementx lines

* Update ads.amp.esm.html

removed whitespaces on both the lines

* Update incrementx.js
eszponder pushed a commit to krzysztofequativ/amphtml that referenced this pull request Apr 22, 2024
@danielrozenberg @eszponder
✅ Fix validator tests broken by ampproject#39237 (ampproject#39298)
12d1a20
eszponder pushed a commit to krzysztofequativ/amphtml that referenced this pull request Apr 22, 2024
@banaag @eszponder
Validator rollup (ampproject#39427)
f554a14 
* cl/552881955 Two-way sync for PR ampproject#39237. No-op, or fixes merge conflicts, if any.

* cl/555542556 n/a

* Updates ads.out to make validator-tests.js pass

* Update some files that fail lint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erwinmombay erwinmombay erwinmombay approved these changes

@powerivq powerivq powerivq approved these changes

@banaag banaag Awaiting requested review from banaag

Assignees
No one assigned
Labels
PR use: In Beta / Experimental PR use: In LTS PR use: In Stable
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Prebid-Vertoz @CLAassistant @powerivq @erwinmombay @ampprojectbot