BAM-2584 AMP-Smartlinks#3
Merged
PhilWinchester merged 16 commits intoamp-smartlinks-0.1from Jan 23, 2019
Merged
Conversation
csdev
approved these changes
Jan 11, 2019
csdev
left a comment
There was a problem hiding this comment.
LGTM pending minor fixes and additional review
extensions/amp-smartlinks/0.1/link-rewriter/link-replacement-cache.js
Outdated
Show resolved
Hide resolved
extensions/amp-smartlinks/0.1/test/validator-amp-smartlinks.out
Outdated
Show resolved
Hide resolved
| ></amp-smartlinks> | ||
|
|
||
|
|
||
| <div class="linkmate-eligible-links"> |
5416408 to
4b24aca
Compare
myfashionhub
approved these changes
Jan 14, 2019
4b24aca to
e51171e
Compare
3c509e5 to
1536438
Compare
c-nichols
reviewed
Jan 16, 2019
| - PhilWinchester | ||
| - pbecotte | ||
| - c-nichols | ||
| - zhouyx |
Author
There was a problem hiding this comment.
yep. Threw the responder to the issue in for now but will probably change later
c-nichols
approved these changes
Jan 17, 2019
1efd884 to
21078bd
Compare
21078bd to
c10ecd8
Compare
PhilWinchester
added a commit
that referenced
this pull request
Jan 24, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
pushed a commit
that referenced
this pull request
Jan 25, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
pushed a commit
that referenced
this pull request
Jan 31, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
pushed a commit
that referenced
this pull request
Jan 31, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
pushed a commit
that referenced
this pull request
Feb 14, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
pushed a commit
that referenced
this pull request
Feb 26, 2019
* adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags
PhilWinchester
added a commit
that referenced
this pull request
Mar 18, 2019
* creating new amp-smartlinks extension * BAM-2584 AMP-Smartlinks (#3) * adding example page and amp-smartlinks to bundles.config * creating amp-smartlinks scaffolding * BAM-2584 adding hardcoded POC implementation of amp-smartlinks * adding new constants file and linkmate file/class * adding linkmate call and association working with link-rewrite service * add amp config call and update workflow to use those values * setup initial amp-smartlinks and linkmate workflow * adding new config variables for link attribute and selector * add tests for amp-smartlinks * add test for linkmate.js * add more thorough anchorList check in runSmartlinks * adding code comments, new constants structure and better options validation * update amp-smartlinks to have helpful information * updating global vars and cleaning up main files * clean up tests and add more explicit type assertions * clean up jsdoc tags * BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (#4) * BAM-2585 whitelist navigation import and fix type errors * replace user.assert with userAssert * BAM-2585 fix validator and type check (#5) * more validator fixes (#6) * BAM-2585 Fix validator, copyright, and whitespace (#7) * alphabetize validator, fix whitespace, and add valid tag * update year in copyright statement * BAM-2585 move `link-rewriter` to import statements and updating types (#8) * BAM-2585 remove link-rewriter and switch to importing from skimlinks extension * clean up promise chain and more descriptive API comments * update xhr to pull from ampdoc.win and add types to constants.js * fix type in buildPageImpressionPayload_ * update validator with new empty value check (#9) * update validator for exclusive-links * switch page-impression API request to customEventReporter (#11) * BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (#12) * fix jsdoc for linkmate params and make runLinkmate more readable (#13) * add try/catch on amp_config fetch and updated constants.js * remove bad type in constants.js and add check for existing shop-links (#14) * add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID * fix type notation in constants.js and linkmate-options.js * fix indentation in example file * add note to README describing link-rewriter priority queue behavior (#15) * add exception to compile.js for amp-skimlinks * update validator to use empty value as indicator linkmate param * fix validator and linkmate-options to use new config style (#16) * updating tests for linkmate-options.js * remove redundant userAssert in linkmate-options * update tests to reflect config changes * update tests to send accurate config params * update readme to refelct config changes and more accurate function names in linkmate-options
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Putting this up now for early review. There are a few things that still need to be changed but the majority of the behavior is set. Anything under
link-rewriteris the "shared" functionality with Skimlinks and can't be touched.TODO
FixrunSmartlinksto correctly detect change in anchorList.Insert our extensionsLinkRewriterwith priority.Add better validation togetConfigOptions.