Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Added EntityHints unit tests#3524

Merged
redmunds merged 5 commits into
adobe:masterfrom
WebsiteDeveloper:EntityHintsUnitTests
May 3, 2013
Merged

Added EntityHints unit tests#3524
redmunds merged 5 commits into
adobe:masterfrom
WebsiteDeveloper:EntityHintsUnitTests

Conversation

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor

No description provided.

@ghost ghost assigned redmunds Apr 30, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: please add newline at end of file

@redmunds

redmunds commented May 1, 2013

Copy link
Copy Markdown
Contributor

Done with initial code review. This looks good. Some suggestions for a few more tests, and a few nits.

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor Author

@redmunds changes pushed.

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor Author

@redmunds fixes pushed.
By the way i don't get notifications somehow when you comment directly on the commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(moving this comment from commit to pull request).

This test should be: "Should show HTML hints after HTML Entity on same line".

@redmunds

redmunds commented May 3, 2013

Copy link
Copy Markdown
Contributor

@WebsiteDeveloper Sorry for commenting on the commit.

Done with review.

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor Author

@redmunds fixes pushed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should test that HTML (property) code hints are showing, as opposed to HTML Entity code hints are not showing. But, since HTMLCodeHints are implemented as an extension, there's no guarantee that they will exist, so you can't test that here. That test needs to be done in the HTMLCodeHints extension, so this test should be removed. Sorry for the confusion.

@redmunds

redmunds commented May 3, 2013

Copy link
Copy Markdown
Contributor

@WebsiteDeveloper Done with review.

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor Author

@redmunds remove the test.
I think i'll put up a pull to add the test to HTMLCodeHints

@WebsiteDeveloper

Copy link
Copy Markdown
Contributor Author

@redmunds put up a pull to add the test to HTMLCodeHints in case you want to do a quick review.

@redmunds

redmunds commented May 3, 2013

Copy link
Copy Markdown
Contributor

Looks good. Merging.

redmunds added a commit that referenced this pull request May 3, 2013
@redmunds redmunds merged commit c3b8fd3 into adobe:master May 3, 2013
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.

2 participants