Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Image and link methods #625

Merged
merged 20 commits into from
Nov 28, 2014
Merged

Image and link methods #625

merged 20 commits into from
Nov 28, 2014

Conversation

mightyiam
Copy link
Contributor

Depends on #627.

}
});
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was moved to a dedicated images.js test file.

@mightyiam
Copy link
Contributor Author

This releases some pressure from the #512 PR.


module("images", {setup: prepareUnitTestModule});

test("Image insertion in a paragraph", function () {

Choose a reason for hiding this comment

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

@winhamwr can weigh in on this, but what do think of having a verb and an implied boolean outcome in the test name, e.g. "Can insert image into a paragraph"?

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the verb and boolean name. "Image insertion in a paragraph" is a bit ambiguous. It could be testing that you can't insert an image or that something specific happens when you do the insert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. "Inserts image in a paragraph".


module("links", {setup: prepareUnitTestModule});

test("Insert link with some attributes.", function () {

Choose a reason for hiding this comment

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

Inserting link with attributes succeeds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Inserts link with attributes".

@winhamwr
Copy link
Member

Looks like there are some test name improvements to be made and some variable name cleanup. Looks good to me, once those are done.

mightyiam added a commit that referenced this pull request Nov 28, 2014
@mightyiam mightyiam merged commit cdd2fb8 into master Nov 28, 2014
@mightyiam mightyiam deleted the imageAndLink branch November 28, 2014 13:36
@mightyiam
Copy link
Contributor Author

I'm not sure how we got to this, but some comments by @michaelgzoller in the PR are about changes that seem to be already merge. Probably from another PR, which was merged into this.

Since it is about test and variable names and it is already merged, I'll go ahead and merge this and fix these names in master, even.

@mightyiam
Copy link
Contributor Author

I updated these test names in master.

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.

3 participants