Skip to content

chore(livesync): make livesync method public #1388

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.

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

Merged
merged 4 commits into from
Jun 26, 2018
Merged

Conversation

ADjenkov
Copy link
Contributor

Livesync method is used in the e2e/modal-navigaiton-ng applicaiton when resetting the root view of the application

@@ -210,7 +210,7 @@ export class NativeScriptPlatformRef extends PlatformRef {
}

@profile
private livesync() {
public livesync() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be public livesync or _livesync?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really not sure. @vakrilov what do you think? If it is lower case _livesync no need to document it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to make it public _livesync() for now

@ghost ghost added the in progress label Jun 22, 2018
vakrilov
vakrilov previously approved these changes Jun 25, 2018
Copy link
Contributor

@vakrilov vakrilov left a comment

Choose a reason for hiding this comment

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

Approved: once the livesync is renamed to _livesync

@ADjenkov ADjenkov merged commit bdd9ece into master Jun 26, 2018
@ghost ghost removed the in progress label Jun 26, 2018
@ADjenkov ADjenkov deleted the djenkov/public-livesync branch June 26, 2018 10:33
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