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

Add a newInstance method for creating a validator instance, #5534

Merged
merged 2 commits into from Oct 12, 2016
Merged

Add a newInstance method for creating a validator instance, #5534

merged 2 commits into from Oct 12, 2016

Conversation

powdercloud
Copy link
Contributor

which is synchronous and much simpler than the existing getInstance method
(it's just a constructor call thus far).

which is synchronous and much simpler than the existing getInstance method
(it's just a constructor call thus far).
@powdercloud
Copy link
Contributor Author

The point of this is that with this change, I should then be able to replace most of this file https://github.com/ampproject/ampbench/blob/master/ampbench_validator.js with a call to newInstance. Alternative would be to expose the constructor of the Validator class, but I think the creation method is useful as a little hedge that allows future changes.

@powdercloud powdercloud merged commit 42b108d into ampproject:master Oct 12, 2016
@powdercloud powdercloud deleted the instance branch October 12, 2016 17:25
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
…ct#5534)

* Add a newInstance method for creating a validator instance,
which is synchronous and much simpler than the existing getInstance method
(it's just a constructor call thus far).

* fix typo
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
…ct#5534)

* Add a newInstance method for creating a validator instance,
which is synchronous and much simpler than the existing getInstance method
(it's just a constructor call thus far).

* fix typo
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.

None yet

5 participants