Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

feat: Removes Renderer2 to be compatible with ivy. #542

Merged
merged 2 commits into from
Feb 7, 2020

Conversation

thomaspink
Copy link
Contributor

@thomaspink thomaspink commented Feb 4, 2020

Pull Request

Removes Renderer2 to be compatible with ivy.
Closes #518

BREAKING CHANGES:

  • Components that are using the outdated Renderer2, which is removed in this commit, do this by injecting it via DI in their constructor. When removing the renderer constructor param the api changes which leads to a breaking change.
  • Changed core platform utils to be internal

Type of PR

Breaking change (fix or change that would cause existing functionality to not work as expected)

Checklist

  • I have read the CONTRIBUTING doc and I follow the PR guidelines
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@github-actions
Copy link

github-actions bot commented Feb 4, 2020

Deploy preview for website ready!

Built with commit 4f56231

https://barista-iwsxegig4.now.sh

tomheller
tomheller previously approved these changes Feb 4, 2020
ffriedl89
ffriedl89 previously approved these changes Feb 5, 2020
components/core/src/util/platform-util.ts Outdated Show resolved Hide resolved
@@ -693,7 +692,7 @@ class TestApp {
{ col1: 'test 1', col2: 'test 2', col3: 'test 3' },
];

constructor(public _renderer: Renderer2) {}
constructor() {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: Can be removed

lukasholzer
lukasholzer previously approved these changes Feb 5, 2020
lara-aigmueller
lara-aigmueller previously approved these changes Feb 5, 2020
lara-aigmueller
lara-aigmueller previously approved these changes Feb 5, 2020
@tomheller tomheller added the pr: needs-rebase This PR needs rebasing label Feb 5, 2020
tomheller
tomheller previously approved these changes Feb 5, 2020
Copy link
Collaborator

@tomheller tomheller left a comment

Choose a reason for hiding this comment

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

Code looks good, but there are some conflicts.

rowa-audil
rowa-audil previously approved these changes Feb 5, 2020
Copy link
Contributor

@rowa-audil rowa-audil left a comment

Choose a reason for hiding this comment

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

code owner file LGTM

@thomaspink thomaspink removed the pr: needs-rebase This PR needs rebasing label Feb 6, 2020
rowa-audil
rowa-audil previously approved these changes Feb 6, 2020
Closes #518

BREAKING CHANGE: Components that are using the outdated Renderer2, which is removed in this commit, do this by injecting it via DI in their constructor. When removing the renderer constructor param the api changes which leads to a breaking change.
BREAKING CHANGE: Core platform utils changed to be internal.
@sonarcloud
Copy link

sonarcloud bot commented Feb 6, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

LGTM

@thomaspink thomaspink added the pr: merge-ready This PR is ready to be merged label Feb 7, 2020
@github-actions github-actions bot added the target: major This PR is targeted for the next major release label Feb 7, 2020
@thomaspink thomaspink merged commit a5f015b into master Feb 7, 2020
@thomaspink thomaspink deleted the ivy-remove-renderer2 branch February 7, 2020 08:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: merge-ready This PR is ready to be merged target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ivy: Remove Renderer2 occurrences
7 participants