Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

refactor(renderer): Extract the renderer to a new library #30

Merged
merged 14 commits into from
Nov 28, 2016

Conversation

danpersa
Copy link
Contributor

affects: tessellate-fragment, tessellate-renderer

affects: tessellate-fragment, tessellate-renderer
@mfellner
Copy link
Contributor

Looks good so far. I'd like to add a commit to clean up the dependencies a bit and to fix the failing tests.

@danpersa
Copy link
Contributor Author

Please go ahead :)

Maximilian Fellner added 12 commits November 24, 2016 15:16
affects: tessellate-renderer
affects: tessellate-renderer
affects: tessellate-renderer
affects: tessellate-renderer
affects: tessellate-renderer

Tessellate libraries should be named with simple verbs or nouns but not with verbs used as nouns.

Tessellate services should be named with any nouns.
affects: tessellate-fragment
affects: tessellate-render
affects: tessellate-fragment, tessellate-render
affects: tessellate-fragment
affects: tessellate-render
affects: tessellate-render
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c822205 on extract-tesselate-renderer into ** on master**.

Copy link
Contributor

@mfellner mfellner left a comment

Choose a reason for hiding this comment

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

  • Added test snapshots
  • Fixed local dependency resolution with Lerna
  • Temporarily removed logger

@@ -1 +1,2 @@
/dist
test/__snapshots__
Copy link
Contributor

Choose a reason for hiding this comment

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

Jest snapshots must be included in the repository. Otherwise running the test is always going to create a new snapshot from scratch and not validate against the existing one. See also http://facebook.github.io/jest/docs/tutorial-react-native.html#snapshot-test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in aa9a580

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -37,7 +37,8 @@
"react": "15.3.2",
"react-dom": "15.3.2",
"request": "2.78.0",
"request-promise-native": "1.0.3"
"request-promise-native": "1.0.3",
"tessellate-renderer": "../tessellate-renderer"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should declare this dependency absolutely and let Lerna handle the resolution: https://github.com/lerna/lerna#how-bootstrap-works.

warn: debug(`${PREFIX}:warn:${name}`),
error: debug(`${PREFIX}:error:${name}`)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest completely removing logger.js (and logging) from this library for now and later re-add it via #31

@danpersa
Copy link
Contributor Author

👍

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9be9941 on extract-tesselate-renderer into ** on master**.

@danpersa danpersa merged commit 4de88ad into master Nov 28, 2016
@mfellner mfellner deleted the extract-tesselate-renderer branch November 28, 2016 10:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants