Skip to content
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

Small refactoring #36

Closed
wants to merge 2 commits into from
Closed

Small refactoring #36

wants to merge 2 commits into from

Conversation

churkin
Copy link
Contributor

@churkin churkin commented Aug 10, 2015

\cc @inikulin

I think it is more logical.

@inikulin
Copy link
Contributor

Tests fail due to incorrect ES6 syntax

@inikulin
Copy link
Contributor

Moreover more likely es7.classProperties will not make it (see: babel/babel#1990), so there will be a task to remove them completely from the codebase.

@inikulin inikulin closed this Aug 10, 2015
@inikulin
Copy link
Contributor

Issued a bug: #37. @churkin Can you do the same for the hammerhead?

@churkin
Copy link
Contributor Author

churkin commented Aug 10, 2015

Of course DevExpress/testcafe-hammerhead#29

@inikulin
Copy link
Contributor

I think since it's constants anyway we will replace it with the instance vars, e.g.:
static TIMEOUT = 123; -> this.TIMEOUT = 123;. It's also will simplify some tests, since we will not need to restore constant var value.

@churkin
Copy link
Contributor Author

churkin commented Aug 10, 2015

In this case "var instanceCount = 0;" is a real static field :)

@inikulin
Copy link
Contributor

Yes, this case is an exception. But currently we've used static fields only for the constants which can be modified for testing purposes.

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.

2 participants