Skip to content

potench/upgrade cmp 0.0.1#5

Merged
potench merged 7 commits into
masterfrom
potench/upgrade-cmp-0.0.1
Aug 7, 2018
Merged

potench/upgrade cmp 0.0.1#5
potench merged 7 commits into
masterfrom
potench/upgrade-cmp-0.0.1

Conversation

@potench
Copy link
Copy Markdown

@potench potench commented Aug 7, 2018

Background

  • Upgrade CMP from upstream appnexus/cmp master

Test Plan

yarn test

by pubvendors.json.
</span>
</span>
<span class={style.argument}>
Copy link
Copy Markdown

@danielrussellLA danielrussellLA Aug 7, 2018

Choose a reason for hiding this comment

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

why is the spacing so large in these files?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nvm it's an upstream merge i see

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

project is using tabs I guess this is how github renders the tabs, indentation looks right on my IDE though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

also, even if it's not conforming to our standard, i'd leave this as is so it's easier to manage upstream changes in the cmp (trying to touch as little of the core of the project as possible)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@potench potench merged commit 52176d1 into master Aug 7, 2018
@potench potench deleted the potench/upgrade-cmp-0.0.1 branch August 20, 2018 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants